From a43cf5c02f9c22e3936101c21529e8fa94cc12e4 Mon Sep 17 00:00:00 2001 From: Mark Egan-Fuller Date: Thu, 14 Jul 2022 13:46:42 +0100 Subject: [PATCH 1/5] Expand tests to include confirmation of onlyif and command. --- spec/defines/init_spec.rb | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/spec/defines/init_spec.rb b/spec/defines/init_spec.rb index 2d91334..ed8eb0a 100644 --- a/spec/defines/init_spec.rb +++ b/spec/defines/init_spec.rb @@ -16,6 +16,35 @@ 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 /aix/, /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 /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 end end end From d185f7d73858fe5b2dd1bc288dfea2a781419cb8 Mon Sep 17 00:00:00 2001 From: Mark Egan-Fuller Date: Thu, 14 Jul 2022 16:37:17 +0100 Subject: [PATCH 2/5] Add beaker tests. --- Gemfile | 5 ++ spec/acceptance/nodesets/default.yml | 10 +++ .../recursive_file_permissions_define_spec.rb | 63 +++++++++++++++++++ spec/spec_helper_acceptance.rb | 23 +++++++ 4 files changed, 101 insertions(+) create mode 100644 spec/acceptance/nodesets/default.yml create mode 100644 spec/acceptance/recursive_file_permissions_define_spec.rb create mode 100644 spec/spec_helper_acceptance.rb 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/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..f372e06 --- /dev/null +++ b/spec/acceptance/recursive_file_permissions_define_spec.rb @@ -0,0 +1,63 @@ +require 'spec_helper_acceptance' + +modulepath = '/etc/puppet/modules' + +describe 'recursive_file_permissions' do + context 'with basic params' do + manifest = <<-EOS + recursive_file_permissions { '/tmp/blah': + file_mode => '0644', + dir_mode => '0744', + owner => 'test', + group => 'test', + } + EOS + + before(:all) do + on(hosts, 'mkdir -p /tmp/blah') + on(hosts, 'mkdir -p /tmp/blah/dirmd') + on(hosts, 'touch /tmp/blah/filemd') + on(hosts, 'touch /tmp/blah/own') + on(hosts, 'touch /tmp/blah/grp') + # Exit 0 here allows us to rerun tests without destroying the boxes + on(hosts, 'useradd bob || exit 0') + on(hosts, 'useradd test || exit 0') + end + + before(:each) do + on(hosts, 'chown bob:test /tmp/blah/own') + on(hosts, 'chown test:bob /tmp/blah/grp') + on(hosts, 'chmod -R 777 /tmp/blah') + end + + 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(on(hosts, "stat /tmp/blah/own --format '%U'")[0].output).to eq "bob\n" + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(on(hosts, "stat /tmp/blah/own --format '%U'")[0].output).to eq "test\n" + end + + it 'changes the group' do + expect(on(hosts, "stat /tmp/blah/grp --format '%G'")[0].output).to eq "bob\n" + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(on(hosts, "stat /tmp/blah/grp --format '%G'")[0].output).to eq "test\n" + end + + it 'changes the dir mode' do + expect(on(hosts, "stat /tmp/blah/dirmd --format '%a'")[0].output).to eq "777\n" + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(on(hosts, "stat /tmp/blah/dirmd --format '%a'")[0].output).to eq "744\n" + end + + it 'changes the file mode' do + expect(on(hosts, "stat /tmp/blah/filemd --format '%a'")[0].output).to eq "777\n" + apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) + expect(on(hosts, "stat /tmp/blah/filemd --format '%a'")[0].output).to eq "644\n" + 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 From 6af08e97bb00f4754dfd039a8a7dbc2a53939fb5 Mon Sep 17 00:00:00 2001 From: Mark Egan-Fuller Date: Fri, 15 Jul 2022 14:35:33 +0100 Subject: [PATCH 3/5] Add ignore_paths functionality. Normally you can just specify a file within a managed directory as a separate file resource to adjust its permissions separately, but this doesn't work with recursive_file_permissions. This adds functionality to specifically ignore paths to allow them to be managed separately. --- README.md | 29 ++ manifests/init.pp | 35 ++- .../recursive_file_permissions_define_spec.rb | 255 ++++++++++++++++-- spec/defines/init_spec.rb | 124 ++++++++- 4 files changed, 401 insertions(+), 42 deletions(-) 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..459d273 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 = 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/recursive_file_permissions_define_spec.rb b/spec/acceptance/recursive_file_permissions_define_spec.rb index f372e06..4ceded1 100644 --- a/spec/acceptance/recursive_file_permissions_define_spec.rb +++ b/spec/acceptance/recursive_file_permissions_define_spec.rb @@ -2,34 +2,223 @@ 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 params' 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 - before(:all) do - on(hosts, 'mkdir -p /tmp/blah') - on(hosts, 'mkdir -p /tmp/blah/dirmd') - on(hosts, 'touch /tmp/blah/filemd') - on(hosts, 'touch /tmp/blah/own') - on(hosts, 'touch /tmp/blah/grp') - # Exit 0 here allows us to rerun tests without destroying the boxes - on(hosts, 'useradd bob || exit 0') - on(hosts, 'useradd test || exit 0') + 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 - on(hosts, 'chown bob:test /tmp/blah/own') - on(hosts, 'chown test:bob /tmp/blah/grp') - on(hosts, 'chmod -R 777 /tmp/blah') + 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 }) @@ -37,27 +226,51 @@ end it 'changes the owner' do - expect(on(hosts, "stat /tmp/blah/own --format '%U'")[0].output).to eq "bob\n" + expect(stat_owner("#{dir}/own")).to eq 'bob' apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) - expect(on(hosts, "stat /tmp/blah/own --format '%U'")[0].output).to eq "test\n" + expect(stat_owner("#{dir}/own")).to eq 'test' end it 'changes the group' do - expect(on(hosts, "stat /tmp/blah/grp --format '%G'")[0].output).to eq "bob\n" + expect(stat_group("#{dir}/grp")).to eq 'bob' apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) - expect(on(hosts, "stat /tmp/blah/grp --format '%G'")[0].output).to eq "test\n" + expect(stat_group("#{dir}/grp")).to eq 'test' end it 'changes the dir mode' do - expect(on(hosts, "stat /tmp/blah/dirmd --format '%a'")[0].output).to eq "777\n" + expect(stat_mode("#{dir}/dirmd")).to eq '777' apply_manifest(manifest, { catch_failures: true, modulepath: modulepath }) - expect(on(hosts, "stat /tmp/blah/dirmd --format '%a'")[0].output).to eq "744\n" + expect(stat_mode("#{dir}/dirmd")).to eq '744' end it 'changes the file mode' do - expect(on(hosts, "stat /tmp/blah/filemd --format '%a'")[0].output).to eq "777\n" + 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(on(hosts, "stat /tmp/blah/filemd --format '%a'")[0].output).to eq "644\n" + 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 ed8eb0a..8aa39de 100644 --- a/spec/defines/init_spec.rb +++ b/spec/defines/init_spec.rb @@ -19,32 +19,140 @@ 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 '.*'", + "find /tmp/blah \"(\" \"(\" -type f '!' -perm 0644 \")\" -o \"(\" -type d '!' -perm 0744 \")\" -o \"(\" '!' -user test \")\" -o \"(\" '!' -group test \")\" \")\" | grep '.*'", ) end case os - when /aix/, /solaris/ + 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 {} \\;", + "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 /darwin/ + 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 {} \\;", + "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 {} \\;", + "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 From 0fc69d5e0370011bedea343772e01d97c4bfd43d Mon Sep 17 00:00:00 2001 From: Mark Egan-Fuller Date: Fri, 15 Jul 2022 15:02:16 +0100 Subject: [PATCH 4/5] Rubocop fixes * Ignore line length in `spec/defines/init_spec.rb` as we have a lot of long string literals * Remove extra whitespace --- .rubocop.yml | 2 ++ spec/defines/init_spec.rb | 1 - 2 files changed, 2 insertions(+), 1 deletion(-) 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/spec/defines/init_spec.rb b/spec/defines/init_spec.rb index 8aa39de..556c43b 100644 --- a/spec/defines/init_spec.rb +++ b/spec/defines/init_spec.rb @@ -152,7 +152,6 @@ end end end - end end end From 8ef24547925f51d0d75e6423c0077c8f860b18eb Mon Sep 17 00:00:00 2001 From: Mark Egan-Fuller Date: Fri, 15 Jul 2022 15:16:25 +0100 Subject: [PATCH 5/5] Use internal join so it works on puppet 4.6.1 --- manifests/init.pp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manifests/init.pp b/manifests/init.pp index 459d273..2969a37 100644 --- a/manifests/init.pp +++ b/manifests/init.pp @@ -75,7 +75,7 @@ $ignore_path_args = case $ignore_paths { undef: { '' } default: { - $ignore_path_join = join($ignore_paths.map |$path| { shellquote('(', '!', '-path', $path, ')') }, ' -a ') + $ignore_path_join = recursive_file_permissions::join($ignore_paths.map |$path| { shellquote('(', '!', '-path', $path, ')') }, ' -a ') "-a ${ignore_path_join}" } }