diff --git a/.rubocop.yml b/.rubocop.yml index 858882d..ce31728 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -21,6 +21,8 @@ AllCops: Metrics/LineLength: Description: People have wide screens, use them. Max: 200 + Exclude: + - spec/defines/init_spec.rb GetText: Enabled: false GetText/DecorateString: diff --git a/Gemfile b/Gemfile index 8007ad0..cdc2038 100644 --- a/Gemfile +++ b/Gemfile @@ -30,6 +30,11 @@ group :development do gem "puppet-module-win-dev-r#{minor_version}", '~> 0.4', require: false, platforms: [:mswin, :mingw, :x64_mingw] end +group :acceptance do + gem 'beaker-rspec' + gem 'beaker-vagrant' +end + puppet_version = ENV['PUPPET_GEM_VERSION'] facter_version = ENV['FACTER_GEM_VERSION'] hiera_version = ENV['HIERA_GEM_VERSION'] diff --git a/README.md b/README.md index be64952..f6eaf3b 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,35 @@ recursive_file_permissions { '/my_dir': } ``` +### Ignoring Paths + +Normally you can just specify a file within a managed directory as a separate +file resource to adjust its permissions separately, but due to the way +recursive_file_permissions works it's necessary to explicitly ignore paths: + +```puppet +recursive_file_permissions { '/my_dir': + owner => 'me', + ignore_paths => [ '/my_dir/stuff/*' ] +} +``` + +Note that if you want to ignore a directory and its contents both will need +adding to the list: + +```puppet +ignore_paths => [ '/my_dir/this/', '/my_dir/this/*' ] +``` + ## Development PRs welcome. + +### Testing + +``` +# To run spec tests +bundle exec rake spec +# To run beaker acceptance tests (requires vagrant) +bundle exec rake beaker +``` diff --git a/manifests/init.pp b/manifests/init.pp index f57ced9..2969a37 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -6,17 +6,19 @@ # # @example # recursive_file_permissions { '/my_dir': -# file_mode => '0644', -# dir_mode => '0744', -# owner => 'me', -# group => 'us', +# file_mode => '0644', +# dir_mode => '0744', +# owner => 'me', +# group => 'us', +# ignore_paths => ['/my_dir/ignored/*'] # } define recursive_file_permissions( - Recursive_file_permissions::Unixpath $target_dir = $title, - Optional[Recursive_file_permissions::Filemode] $file_mode = undef, - Optional[Recursive_file_permissions::Filemode] $dir_mode = undef, - Optional[String[1]] $owner = undef, - Optional[String[1]] $group = undef, + Recursive_file_permissions::Unixpath $target_dir = $title, + Optional[Recursive_file_permissions::Filemode] $file_mode = undef, + Optional[Recursive_file_permissions::Filemode] $dir_mode = undef, + Optional[String[1]] $owner = undef, + Optional[String[1]] $group = undef, + Optional[Array[Recursive_file_permissions::Unixpath]] $ignore_paths = undef, ) { if $facts['os']['family'] == 'windows' { @@ -29,7 +31,7 @@ # Define the find arguments to find and fix any of the permissions we want to # recursively manage. Each element defines: - # + # # - input. The param this relates to. If not undef, the check will be used. # - find. String. Find args that will identify files in need of fixing. # - fix. String. Find -exec command to fix identified files. @@ -70,15 +72,23 @@ } }.recursive_file_permissions::join(' -o ') + $ignore_path_args = case $ignore_paths { + undef: { '' } + default: { + $ignore_path_join = recursive_file_permissions::join($ignore_paths.map |$path| { shellquote('(', '!', '-path', $path, ')') }, ' -a ') + "-a ${ignore_path_join}" + } + } + # This will become the onlyif commmand to run. - $onlyif = "find ${shellsafe_dir} ${onlyif_find_args} | grep '.*'" + $onlyif = "find ${shellsafe_dir} \"(\" ${onlyif_find_args} \")\" ${ignore_path_args} | grep '.*'" # Build an &&-joined command series to run that will find and fix any # deviation from the desired state of any validator. $command = $validators.reduce([]) |$arr,$validator| { $validator[input] ? { undef => $arr, - default => $arr << "find ${shellsafe_dir} '(' ${validator[find]} ')' ${validator[fix]}" + default => $arr << "find ${shellsafe_dir} \"(\" ${validator[find]} \")\" ${ignore_path_args} ${validator[fix]}" } }.recursive_file_permissions::join(' && ') @@ -90,5 +100,4 @@ onlyif => $onlyif, command => $command, } - } diff --git a/spec/acceptance/nodesets/default.yml b/spec/acceptance/nodesets/default.yml new file mode 100644 index 0000000..f94e678 --- /dev/null +++ b/spec/acceptance/nodesets/default.yml @@ -0,0 +1,10 @@ +--- +HOSTS: + testserver: + roles: + - master + platform: ubuntu-20.04-amd64 + box: ubuntu/focal64 + hypervisor: vagrant +CONFIG: + type: foss diff --git a/spec/acceptance/recursive_file_permissions_define_spec.rb b/spec/acceptance/recursive_file_permissions_define_spec.rb new file mode 100644 index 0000000..4ceded1 --- /dev/null +++ b/spec/acceptance/recursive_file_permissions_define_spec.rb @@ -0,0 +1,276 @@ +require 'spec_helper_acceptance' + +modulepath = '/etc/puppet/modules' + +def setup_test_dir(dir) + on(hosts, "rm -rf '#{dir}'") + + on(hosts, "mkdir -p '#{dir}'") + on(hosts, "mkdir -p '#{dir}/dirmd'") + on(hosts, "mkdir -p '#{dir}/ignore'") + on(hosts, "touch '#{dir}/filemd'") + on(hosts, "touch '#{dir}/own'") + on(hosts, "touch '#{dir}/grp'") + on(hosts, "touch '#{dir}/ignore/filemd'") + on(hosts, "touch '#{dir}/ignore/own'") + on(hosts, "touch '#{dir}/ignore/grp'") +end + +def stat_owner(path) + on(hosts, "stat '#{path}' --format '%U'")[0].output.strip +end + +def stat_group(path) + on(hosts, "stat '#{path}' --format '%G'")[0].output.strip +end + +def stat_mode(path) + on(hosts, "stat '#{path}' --format '%a'")[0].output.strip +end + +# Exit 0 here allows us to rerun tests without destroying the boxes +on(hosts, 'useradd bob || exit 0') +on(hosts, 'useradd test || exit 0') + +shared_context 'common' do + before(:each) do + setup_test_dir(dir) + on(hosts, "chown bob:test '#{dir}/own' '#{dir}/ignore/own'") + on(hosts, "chown test:bob '#{dir}/grp' '#{dir}/ignore/grp'") + on(hosts, "chmod -R 777 '#{dir}'") + end +end + +describe 'recursive_file_permissions' do + context 'with basic parameters' do + let(:dir) { '/tmp/blah' } + + include_context 'common' + + manifest = <<-EOS + recursive_file_permissions { '/tmp/blah': + file_mode => '0644', + dir_mode => '0744', + owner => 'test', + group => 'test', + } + EOS + + it 'is idempotent with no errors' do + # Run it twice and test for idempotency + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + apply_manifest(manifest, { catch_changes: true, modulepath: modulepath }) + end + + it 'changes the owner' do + expect(stat_owner("#{dir}/own")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_owner("#{dir}/own")).to eq 'test' + end + + it 'changes the group' do + expect(stat_group("#{dir}/grp")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_group("#{dir}/grp")).to eq 'test' + end + + it 'changes the dir mode' do + expect(stat_mode("#{dir}/dirmd")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/dirmd")).to eq '744' + end + + it 'changes the file mode' do + expect(stat_mode("#{dir}/filemd")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/filemd")).to eq '644' + end + end + + context 'with ignored_paths' do + let(:dir) { '/tmp/blah' } + + include_context 'common' + + manifest = <<-EOS + recursive_file_permissions { '/tmp/blah': + file_mode => '0644', + dir_mode => '0744', + owner => 'test', + group => 'test', + ignore_paths => ['/tmp/blah/ignore*'] + } + EOS + + it 'is idempotent with no errors' do + # Run it twice and test for idempotency + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + apply_manifest(manifest, { catch_changes: true, modulepath: modulepath }) + end + + it 'changes the owner' do + expect(stat_owner("#{dir}/own")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_owner("#{dir}/own")).to eq 'test' + end + + it 'changes the group' do + expect(stat_group("#{dir}/grp")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_group("#{dir}/grp")).to eq 'test' + end + + it 'changes the dir mode' do + expect(stat_mode("#{dir}/dirmd")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/dirmd")).to eq '744' + end + + it 'changes the file mode' do + expect(stat_mode("#{dir}/filemd")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/filemd")).to eq '644' + end + + it 'doesn\'t change the owner of an ignored path' do + expect(stat_owner("#{dir}/ignore/own")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_owner("#{dir}/ignore/own")).to eq 'bob' + end + + it 'doesn\'t change the group of an ignored path' do + expect(stat_group("#{dir}/ignore/grp")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_group("#{dir}/ignore/grp")).to eq 'bob' + end + + it 'doesn\'t change the dir mode of an ignored path' do + expect(stat_mode("#{dir}/ignore")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/ignore")).to eq '777' + end + + it 'doesn\'t change the file mode of an ignored path' do + expect(stat_mode("#{dir}/ignore/filemd")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/ignore/filemd")).to eq '777' + end + end + + context 'with only ignored_paths changed' do + let(:dir) { '/tmp/blah' } + + before(:each) do + setup_test_dir(dir) + # Ensure other paths are set correctly + on(hosts, "chmod -R 644 #{dir}") + on(hosts, "chmod 744 #{dir} #{dir}/dirmd") + on(hosts, "chown -R test:test #{dir}") + + # Only 'change' ignored paths + on(hosts, "chown bob:test #{dir}/ignore/own") + on(hosts, "chown test:bob #{dir}/ignore/grp") + on(hosts, "chmod -R 777 #{dir}/ignore") + end + + manifest = <<-EOS + recursive_file_permissions { '/tmp/blah': + file_mode => '0644', + dir_mode => '0744', + owner => 'test', + group => 'test', + ignore_paths => ['/tmp/blah/ignore/*', '/tmp/blah/ignore'] + } + EOS + + it 'is idempotent with no errors' do + # Run it twice and test for idempotency + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + apply_manifest(manifest, { catch_changes: true, modulepath: modulepath }) + end + + it 'does not change anything' do + expect(stat_owner("#{dir}/ignore/own")).to eq 'bob' + expect(stat_group("#{dir}/ignore/grp")).to eq 'bob' + expect(stat_mode("#{dir}/ignore")).to eq '777' + expect(stat_mode("#{dir}/ignore/filemd")).to eq '777' + + apply_manifest(manifest, { catch_changes: true, modulepath: modulepath }) + + expect(stat_owner("#{dir}/ignore/own")).to eq 'bob' + expect(stat_group("#{dir}/ignore/grp")).to eq 'bob' + expect(stat_mode("#{dir}/ignore")).to eq '777' + expect(stat_mode("#{dir}/ignore/filemd")).to eq '777' + end + end + + context 'with paths that need quoting' do + let(:dir) { '/tmp/bl $ah' } + + include_context 'common' + + manifest = <<-EOS + recursive_file_permissions { '/tmp/bl $ah': + file_mode => '0644', + dir_mode => '0744', + owner => 'test', + group => 'test', + ignore_paths => ['/tmp/bl $ah/ignore/*', '/tmp/bl $ah/ignore'] + } + EOS + + it 'is idempotent with no errors' do + # Run it twice and test for idempotency + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + apply_manifest(manifest, { catch_changes: true, modulepath: modulepath }) + end + + it 'changes the owner' do + expect(stat_owner("#{dir}/own")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_owner("#{dir}/own")).to eq 'test' + end + + it 'changes the group' do + expect(stat_group("#{dir}/grp")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_group("#{dir}/grp")).to eq 'test' + end + + it 'changes the dir mode' do + expect(stat_mode("#{dir}/dirmd")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/dirmd")).to eq '744' + end + + it 'changes the file mode' do + expect(stat_mode("#{dir}/filemd")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/filemd")).to eq '644' + end + + it 'doesn\'t change the owner of an ignored path' do + expect(stat_owner("#{dir}/ignore/own")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_owner("#{dir}/ignore/own")).to eq 'bob' + end + + it 'doesn\'t change the group of an ignored path' do + expect(stat_group("#{dir}/ignore/grp")).to eq 'bob' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_group("#{dir}/ignore/grp")).to eq 'bob' + end + + it 'doesn\'t change the dir mode of an ignored path' do + expect(stat_mode("#{dir}/ignore")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/ignore")).to eq '777' + end + + it 'doesn\'t change the file mode of an ignored path' do + expect(stat_mode("#{dir}/ignore/filemd")).to eq '777' + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(stat_mode("#{dir}/ignore/filemd")).to eq '777' + end + end +end diff --git a/spec/defines/init_spec.rb b/spec/defines/init_spec.rb index 2d91334..556c43b 100644 --- a/spec/defines/init_spec.rb +++ b/spec/defines/init_spec.rb @@ -16,6 +16,142 @@ let(:facts) { os_facts } it { is_expected.to compile } + + it 'creates the appropriate onlyif command' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_onlyif( + "find /tmp/blah \"(\" \"(\" -type f '!' -perm 0644 \")\" -o \"(\" -type d '!' -perm 0744 \")\" -o \"(\" '!' -user test \")\" -o \"(\" '!' -group test \")\" \")\" | grep '.*'", + ) + end + + case os + when %r{aix}, %r{solaris} + it 'creates the appropriate command (AIX, Solaris)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" -type f '!' -perm 0644 \")\" \")\" -exec chmod 0644 {} \\; && find /tmp/blah \"(\" \"(\" -type d '!' -perm 0744 \")\" \")\" -exec chmod 0744 {} \\; && find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -exec chown -h test {} \\; && find /tmp/blah \"(\" \"(\" '!' -group test \")\" \")\" -exec chgrp -h test {} \\;", + ) + end + when %r{darwin} + it 'creates the appropriate command (Darwin)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" -type f '!' -perm 0644 \")\" \")\" -exec chmod -v 0644 {} \\; && find /tmp/blah \"(\" \"(\" -type d '!' -perm 0744 \")\" \")\" -exec chmod -v 0744 {} \\; && find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -exec chown -h -v test {} \\; && find /tmp/blah \"(\" \"(\" '!' -group test \")\" \")\" -exec chgrp -h -v test {} \\;", + ) + end + else + it 'creates the appropriate command (Default)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" -type f '!' -perm 0644 \")\" \")\" -exec chmod -c 0644 {} \\; && find /tmp/blah \"(\" \"(\" -type d '!' -perm 0744 \")\" \")\" -exec chmod -c 0744 {} \\; && find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -exec chown -h -c test {} \\; && find /tmp/blah \"(\" \"(\" '!' -group test \")\" \")\" -exec chgrp -h -c test {} \\;", + ) + end + end + + context 'when ignore_path is set' do + let(:params) do + { + 'owner' => 'test', + 'ignore_paths' => ['/tmp/blah/not_this_one'], + } + end + + it 'creates the appropriate onlyif command' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_onlyif( + "find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path /tmp/blah/not_this_one \")\" | grep '.*'", + ) + end + + case os + when %r{aix}, %r{solaris} + it 'creates the appropriate command (AIX, Solaris)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path /tmp/blah/not_this_one \")\" -exec chown -h test {} \\;", + ) + end + when %r{darwin} + it 'creates the appropriate command (Darwin)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path /tmp/blah/not_this_one \")\" -exec chown -h -v test {} \\;", + ) + end + else + it 'creates the appropriate command (Default)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path /tmp/blah/not_this_one \")\" -exec chown -h -c test {} \\;", + ) + end + end + end + + context 'when ignore_path is set with multiple paths' do + let(:params) do + { + 'owner' => 'test', + 'ignore_paths' => ['/tmp/blah/not_this_one', '/tmp/blah/not_this_one_either'], + } + end + + it 'creates the appropriate onlyif command' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_onlyif( + "find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path /tmp/blah/not_this_one \")\" -a \"(\" '!' -path /tmp/blah/not_this_one_either \")\" | grep '.*'", + ) + end + + case os + when %r{aix}, %r{solaris} + it 'creates the appropriate command (AIX, Solaris)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path /tmp/blah/not_this_one \")\" -a \"(\" '!' -path /tmp/blah/not_this_one_either \")\" -exec chown -h test {} \\;", + ) + end + when %r{darwin} + it 'creates the appropriate command (Darwin)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path /tmp/blah/not_this_one \")\" -a \"(\" '!' -path /tmp/blah/not_this_one_either \")\" -exec chown -h -v test {} \\;", + ) + end + else + it 'creates the appropriate command (Default)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/blah').with_command( + "find /tmp/blah \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path /tmp/blah/not_this_one \")\" -a \"(\" '!' -path /tmp/blah/not_this_one_either \")\" -exec chown -h -c test {} \\;", + ) + end + end + end + + context 'when the paths need quoting' do + let(:title) { '/tmp/bl $ah' } + let(:params) do + { + 'owner' => 'test', + 'ignore_paths' => ['/tmp/bl $ah/not this one', "/tmp/bl \$ah/not this\/one either"], + } + end + + it 'creates the appropriate onlyif command' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/bl $ah').with_onlyif( + "find '/tmp/bl $ah' \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path '/tmp/bl $ah/not this one' \")\" -a \"(\" '!' -path '/tmp/bl $ah/not this/one either' \")\" | grep '.*'", + ) + end + + case os + when %r{aix}, %r{solaris} + it 'creates the appropriate command (AIX, Solaris)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/bl $ah').with_command( + "find '/tmp/bl $ah' \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path '/tmp/bl $ah/not this one' \")\" -a \"(\" '!' -path '/tmp/bl $ah/not this/one either' \")\" -exec chown -h test {} \\;", + ) + end + when %r{darwin} + it 'creates the appropriate command (Darwin)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/bl $ah').with_command( + "find '/tmp/bl $ah' \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path '/tmp/bl $ah/not this one' \")\" -a \"(\" '!' -path '/tmp/bl $ah/not this/one either' \")\" -exec chown -h -v test {} \\;", + ) + end + else + it 'creates the appropriate command (Default)' do + is_expected.to contain_exec('recursive_file_permissions:/tmp/bl $ah').with_command( + "find '/tmp/bl $ah' \"(\" \"(\" '!' -user test \")\" \")\" -a \"(\" '!' -path '/tmp/bl $ah/not this one' \")\" -a \"(\" '!' -path '/tmp/bl $ah/not this/one either' \")\" -exec chown -h -c test {} \\;", + ) + end + end + end end end end diff --git a/spec/spec_helper_acceptance.rb b/spec/spec_helper_acceptance.rb new file mode 100644 index 0000000..d9e504c --- /dev/null +++ b/spec/spec_helper_acceptance.rb @@ -0,0 +1,23 @@ +require 'beaker-puppet' +require 'beaker-rspec' + +# Install Puppet on all hosts +install_puppet_on(hosts) + +RSpec.configure do |c| + module_root = File.expand_path(File.join(File.dirname(__FILE__), '..')) + + c.formatter = :documentation + + c.before :suite do + # Install module to all hosts + hosts.each do |host| + install_dev_puppet_module_on( + host, + source: module_root, + module_name: 'recursive_file_permissions', + target_module_path: '/etc/puppet/modules', + ) + end + end +end