From e76bd24dcb33ab71440633303124ea99d59b7783 Mon Sep 17 00:00:00 2001 From: Enis Inan Date: Thu, 13 Dec 2018 02:10:54 -0800 Subject: (maint) Add more crontab provider tests This commit adds the following tests to the cron resource: * A test to ensure that the crontab provider writes the crontab file of a new user * A test to ensure that the crontab provider fails to write the crontab file of a nonexistent user * A test to ensure that the crontab provider writes an originally unauthorized user's crontab file _if_ Puppet authorizes them in the middle of the run These tests are part of the crontab provider's specifications and should have been added before. --- ...il_to_write_a_nonexistent_users_crontab_spec.rb | 20 ++++ ...users_crontab_if_puppet_authorizes_them_spec.rb | 111 +++++++++++++++++++++ ...users_crontab_after_puppet_creates_them_spec.rb | 59 +++++++++++ 3 files changed, 190 insertions(+) create mode 100644 spec/acceptance/tests/resource/cron/should_fail_to_write_a_nonexistent_users_crontab_spec.rb create mode 100644 spec/acceptance/tests/resource/cron/should_write_an_originally_unauthorized_users_crontab_if_puppet_authorizes_them_spec.rb create mode 100644 spec/acceptance/tests/resource/cron/should_write_new_users_crontab_after_puppet_creates_them_spec.rb (limited to 'spec/acceptance/tests') diff --git a/spec/acceptance/tests/resource/cron/should_fail_to_write_a_nonexistent_users_crontab_spec.rb b/spec/acceptance/tests/resource/cron/should_fail_to_write_a_nonexistent_users_crontab_spec.rb new file mode 100644 index 0000000..40fbbd9 --- /dev/null +++ b/spec/acceptance/tests/resource/cron/should_fail_to_write_a_nonexistent_users_crontab_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper_acceptance' + +RSpec.context 'when Puppet attempts to write the crontab of a nonexistent user' do + let(:nonexistent_username) { "pl#{rand(999_999).to_i}" } + + before(:each) do + step 'Ensure that the nonexistent user does not exist' do + compatible_agents.each do |agent| + user_absent(agent, nonexistent_username) + end + end + end + + compatible_agents.each do |agent| + it "should fail on #{agent}" do + manifest = cron_manifest('second_entry', command: 'ls', user: nonexistent_username) + apply_manifest_on(agent, manifest, expect_failures: true) + end + end +end diff --git a/spec/acceptance/tests/resource/cron/should_write_an_originally_unauthorized_users_crontab_if_puppet_authorizes_them_spec.rb b/spec/acceptance/tests/resource/cron/should_write_an_originally_unauthorized_users_crontab_if_puppet_authorizes_them_spec.rb new file mode 100644 index 0000000..964cc17 --- /dev/null +++ b/spec/acceptance/tests/resource/cron/should_write_an_originally_unauthorized_users_crontab_if_puppet_authorizes_them_spec.rb @@ -0,0 +1,111 @@ +require 'spec_helper_acceptance' + +RSpec.context 'when Puppet authorizes a previously unauthorized user to use crontab' do + class << self + alias_method :orig_compatible_agents, :compatible_agents + + # This test is confined to AIX and Solaris agents only. + def compatible_agents + orig_compatible_agents.select do |agent| + agent['platform'].include?('aix') || agent['platform'].include?('solaris') + end + end + end + + let(:username) { "pl#{rand(999_999).to_i}" } + let(:unauthorized_username) { "pl#{rand(999_999).to_i}" } + + let(:cron_deny_path) do + {} + end + let(:cron_deny_original_contents) do + {} + end + + before(:each) do + step 'Ensure that the test users exist' do + compatible_agents.each do |agent| + user_present(agent, username) + user_present(agent, unauthorized_username) + end + end + + step 'Get the current cron.deny contents for each agent' do + compatible_agents.each do |agent| + case agent['platform'] + when %r{aix} + cron_deny_path[agent] = '/var/adm/cron/cron.deny' + when %r{solaris} + cron_deny_path[agent] = '/etc/cron.d/cron.deny' + else + fail_test "Cannot figure out the path of the cron.deny file for the #{agent['platform']} platform" + end + + cron_deny_original_contents[agent] = on(agent, "cat #{cron_deny_path[agent]}").stdout + end + end + end + + after(:each) do + step 'Teardown -- Erase the test users on the agents' do + compatible_agents.each do |agent| + run_cron_on(agent, :remove, username) + user_absent(agent, username) + + run_cron_on(agent, :remove, unauthorized_username) + user_absent(agent, unauthorized_username) + end + end + + # It is possible for the "before" hook to raise an exception while the + # cron.deny contents are being computed, so only execute the next step + # if we've managed to compute the cron.deny contents for each agent + next unless cron_deny_original_contents.keys.size == compatible_agents.size + + step "Teardown -- Reset each agent's cron.deny contents" do + compatible_agents.each do |agent| + apply_manifest_on(agent, file_manifest(cron_deny_path[agent], ensure: :present, content: cron_deny_original_contents[agent])) + end + end + end + + compatible_agents.each do |agent| + it "should write that user's crontab on #{agent}" do + step 'Add the unauthorized user to the cron.deny file' do + on(agent, "echo #{unauthorized_username} >> #{cron_deny_path[agent]}") + end + + step 'Verify that the unauthorized user was added to the cron.deny file' do + cron_deny_contents = on(agent, "cat #{cron_deny_path[agent]}").stdout + assert_match(%r{^#{unauthorized_username}$}, cron_deny_contents, 'Failed to add the unauthorized user to the cron.deny file') + end + + step "Modify the unauthorized user's crontab with Puppet" do + # The scenario we're testing here is: + # * An unrelated cron resource triggers the prefetch step, which will also + # prefetch the crontab of our unauthorized user. The latter prefetch should + # fail, instead returning an empty crontab file. + # + # * Puppet authorizes our unauthorized user by removing them from the cron.deny + # file. + # + # * A cron resource linked to our (originally) unauthorized user should now be able + # to write to that user's crontab file (assuming it requires the resource updating + # the cron.deny file) + # + # The following manifest replicates the above scenario. Note that we test this specific + # scenario to ensure that the changes in PUP-9217 enforce backwards compatibility. + manifest = [ + cron_manifest('first_entry', command: 'ls', user: username), + file_manifest(cron_deny_path[agent], ensure: :present, content: cron_deny_original_contents[agent]), + cron_manifest('second_entry', command: 'ls', user: unauthorized_username), + ].join("\n\n") + apply_manifest_on(agent, manifest) + end + + step "Verify that Puppet did modify the unauthorized user's crontab" do + assert_matching_arrays(['* * * * * ls'], crontab_entries_of(agent, unauthorized_username), "Puppet did not modify the unauthorized user's crontab file") + end + end + end +end diff --git a/spec/acceptance/tests/resource/cron/should_write_new_users_crontab_after_puppet_creates_them_spec.rb b/spec/acceptance/tests/resource/cron/should_write_new_users_crontab_after_puppet_creates_them_spec.rb new file mode 100644 index 0000000..12506ad --- /dev/null +++ b/spec/acceptance/tests/resource/cron/should_write_new_users_crontab_after_puppet_creates_them_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper_acceptance' + +RSpec.context 'when Puppet creates a user in the middle of its run' do + let(:known_username) { "pl#{rand(999_999).to_i}" } + let(:new_username) { "pl#{rand(999_999).to_i}" } + + before(:each) do + step 'Ensure that the known user exists on all of the agents' do + compatible_agents.each do |agent| + user_present(agent, known_username) + end + end + + step 'Ensure that the new user does not exist on all of the agents' do + compatible_agents.each do |agent| + user_absent(agent, new_username) + end + end + end + + after(:each) do + step 'Teardown -- Ensure that both users are deleted on all of the agents' do + compatible_agents.each do |agent| + run_cron_on(agent, :remove, known_username) + user_absent(agent, known_username) + + run_cron_on(agent, :remove, new_username) + user_absent(agent, new_username) + end + end + end + + compatible_agents.each do |agent| + it "should be able to write their crontab on #{agent}" do + puppet_result = nil + step "Create the new user, and the known + new user's crontab entries with Puppet" do + # Placing Cron[first_entry] before creating the new user + # triggers a prefetch of all the Cron resources on the + # system. This lets us test that the prefetch step marks + # the crontab of an unknown user as empty instead of marking + # it as a failure. + manifest = [ + cron_manifest('first_entry', command: 'ls', user: known_username), + user_manifest(new_username, ensure: :present), + cron_manifest('second_entry', command: 'ls', user: new_username), + ].join("\n\n") + puppet_result = apply_manifest_on(agent, manifest) + end + + step 'Verify that Puppet successfully evaluates a Cron resource associated with the new user' do + assert_match(%r{Cron.*second_entry}, puppet_result.stdout, 'Puppet fails to evaluate a Cron resource associated with a new user') + end + + step "Verify that Puppet did create the new user's crontab file" do + assert_matching_arrays(['* * * * * ls'], crontab_entries_of(agent, new_username), "Puppet did not create the new user's crontab file") + end + end + end +end -- cgit v1.2.3 From a369886d68aa84a14defd3e594aa75cab71da328 Mon Sep 17 00:00:00 2001 From: Enis Inan Date: Thu, 13 Dec 2018 02:11:39 -0800 Subject: (MODULES-7789) Port over the PUP-9217 changes PUP-9217 introduced the :raise_prefetch_errors option to the ParsedFile provider base class that, when set, will fail any resources associated with a failed prefetch target. Thus, when a file read error does occur, Puppet will fail all of the resources associated with the failed crontab (target). This means that the failed crontab will not be overwritten, thus fixing the issue described in the ticket. For more details, please refer to https://github.com/puppetlabs/puppet/commit/5b0fa987e5b7b27839e424ff16d59c7bf081c73a --- lib/puppet/provider/cron/crontab.rb | 2 +- ...erwrite_crontab_file_on_file_read_error_spec.rb | 114 ++++++++++++++++++++ ...associated_resources_on_file_read_error_spec.rb | 115 +++++++++++++++++++++ 3 files changed, 230 insertions(+), 1 deletion(-) create mode 100644 spec/acceptance/tests/resource/cron/should_not_overwrite_crontab_file_on_file_read_error_spec.rb create mode 100644 spec/acceptance/tests/resource/cron/should_only_fail_associated_resources_on_file_read_error_spec.rb (limited to 'spec/acceptance/tests') diff --git a/lib/puppet/provider/cron/crontab.rb b/lib/puppet/provider/cron/crontab.rb index 8953109..4ba89e3 100644 --- a/lib/puppet/provider/cron/crontab.rb +++ b/lib/puppet/provider/cron/crontab.rb @@ -1,7 +1,7 @@ require_relative 'filetype' require 'puppet/provider/parsedfile' -Puppet::Type.type(:cron).provide(:crontab, parent: Puppet::Provider::ParsedFile, default_target: ENV['USER'] || 'root') do +Puppet::Type.type(:cron).provide(:crontab, parent: Puppet::Provider::ParsedFile, default_target: ENV['USER'] || 'root', raise_prefetch_errors: true) do commands crontab: 'crontab' text_line :comment, match: %r{^\s*#}, post_parse: proc { |record| diff --git a/spec/acceptance/tests/resource/cron/should_not_overwrite_crontab_file_on_file_read_error_spec.rb b/spec/acceptance/tests/resource/cron/should_not_overwrite_crontab_file_on_file_read_error_spec.rb new file mode 100644 index 0000000..08e8bb8 --- /dev/null +++ b/spec/acceptance/tests/resource/cron/should_not_overwrite_crontab_file_on_file_read_error_spec.rb @@ -0,0 +1,114 @@ +require 'spec_helper_acceptance' + +RSpec.context 'when Puppet cannot read a crontab file' do + let(:username) { "pl#{rand(999_999).to_i}" } + let(:crontab_contents) { '6 6 6 6 6 /usr/bin/true' } + + before(:each) do + step 'Create the user on the agents' do + compatible_agents.each do |agent| + user_present(agent, username) + end + end + + step "Set the user's crontab" do + compatible_agents.each do |agent| + run_cron_on(agent, :add, username, crontab_contents) + assert_matching_arrays([crontab_contents], crontab_entries_of(agent, username), "Could not set the user's crontab for testing") + end + end + end + + after(:each) do + step 'Teardown -- Erase the user on the agents' do + compatible_agents.each do |agent| + run_cron_on(agent, :remove, username) + user_absent(agent, username) + end + end + end + + compatible_agents.each do |agent| + it "should not overwrite it on #{agent}" do + crontab_exe = nil + step 'Find the crontab executable' do + crontab_exe = on(agent, 'which crontab').stdout.chomp + end + + stub_crontab_bin_dir = nil + stub_crontab_exe = nil + step 'Create the stub crontab executable that triggers the read error' do + stub_crontab_bin_dir = agent.tmpdir('stub_crontab_bin_dir') + on(agent, "chown #{username} #{stub_crontab_bin_dir}") + + stub_crontab_exe = "#{stub_crontab_bin_dir}/crontab" + stub_crontab_exe_script = <<-SCRIPT + #!/usr/bin/env bash + exit 1 + SCRIPT + create_remote_file(agent, stub_crontab_exe, stub_crontab_exe_script) + on(agent, "chown #{username} #{stub_crontab_exe}") + on(agent, "chmod a+x #{stub_crontab_exe}") + end + + path_env_var = nil + step 'Get the value of the PATH environment variable' do + path_env_var = on(agent, 'echo $PATH').stdout.chomp + end + + step "(PUP-9217) Attempt to overwrite the user's crontab file" do + # This manifest reproduces the issue in PUP-9217. Here's how: + # 1. When Puppet attempts to realize Cron['first_entry'], it will prefetch + # all of the present cron entries on the system. To do this, it will execute + # the crontab command. Since we prepend stub_crontab_bindir to PATH prior to + # executing Puppet, executing the crontab command will really execute + # stub_crontab_exe, which returns an exit code of 1. This triggers our + # read error. + # + # 2. Puppet will attempt to write Cron['first_entry'] onto disk. However, it will + # fail to do so because it will still execute stub_crontab_exe to perform the + # write. Thus, Cron['first_entry'] will fail its evaluation. + # + # 3. Next, Puppet will modify stub_crontab_exe to now execute the actual crontab + # command so that any subsequent calls to crontab will succeed. Note that under + # the hood, Puppet will still be executing stub_crontab_exe. + # + # 4. Finally, Puppet will attempt to realize Cron['second_entry']. It will skip + # the prefetch step, instead proceeding to directly write Cron['second_entry'] + # to disk. But note that since the prefetch failed in (1), Puppet will proceed + # to overwrite our user's crontab file so that it will contain Cron['first_entry'] + # and Cron['second_entry'] (Cron['first_entry'] is there because Puppet maintains + # each crontab file's entries in memory so that when it writes one entry to disk, + # it will write all of them). + manifest = [ + cron_manifest('first_entry', command: 'ls', user: username), + file_manifest(stub_crontab_exe, content: "#!/usr/bin/env bash\n#{crontab_exe} $@"), + cron_manifest('second_entry', command: 'ls', user: username), + ].join("\n\n") + manifest_file = agent.tmpfile('crontab_overwrite_manifest') + create_remote_file(agent, manifest_file, manifest) + + # We need to run a script here instead of a command because: + # * We need to cd into a directory owned by our user. Otherwise, bash will + # fail to execute stub_crontab_exe on AIX and Solaris because we run crontab + # as the given user, and the given user does not have access to Puppet's cwd. + # + # * We also need to pass-in our PATH to Puppet since it contains stub_crontab_bin_dir. + apply_crontab_overwrite_manifest = agent.tmpfile('apply_crontab_overwrite_manifest') + script = <<-SCRIPT + #!/usr/bin/env bash + + cd #{stub_crontab_bin_dir} && puppet apply #{manifest_file} + SCRIPT + create_remote_file(agent, apply_crontab_overwrite_manifest, script) + on(agent, "chmod a+x #{apply_crontab_overwrite_manifest}") + + on(agent, "bash #{apply_crontab_overwrite_manifest}", environment: { PATH: "#{stub_crontab_bin_dir}:#{path_env_var}" }) + end + + step "(PUP-9217) Verify that Puppet does not overwrite the user's crontab file when it fails to read it" do + assert_matching_arrays([crontab_contents], crontab_entries_of(agent, username), "Puppet overwrote the user's crontab file even though it failed to read it") + end + end + end +end diff --git a/spec/acceptance/tests/resource/cron/should_only_fail_associated_resources_on_file_read_error_spec.rb b/spec/acceptance/tests/resource/cron/should_only_fail_associated_resources_on_file_read_error_spec.rb new file mode 100644 index 0000000..3e06d0c --- /dev/null +++ b/spec/acceptance/tests/resource/cron/should_only_fail_associated_resources_on_file_read_error_spec.rb @@ -0,0 +1,115 @@ +require 'spec_helper_acceptance' + +RSpec.context 'when Puppet cannot read a crontab file' do + let(:username) { "pl#{rand(999_999).to_i}" } + let(:failed_username) { "pl#{rand(999_999).to_i}" } + + before(:each) do + step 'Create the users' do + compatible_agents.each do |agent| + user_present(agent, username) + user_present(agent, failed_username) + end + end + end + + after(:each) do + step 'Teardown -- Erase the users' do + compatible_agents.each do |agent| + run_cron_on(agent, :remove, username) + user_absent(agent, username) + + user_absent(agent, failed_username) + end + end + end + + compatible_agents.each do |agent| + it "should only fail the associated resources on #{agent}" do + crontab_exe = nil + step 'Find the crontab executable' do + crontab_exe = on(agent, 'which crontab').stdout.chomp + end + + stub_crontab_bin_dir = nil + stub_crontab_exe = nil + step 'Create the stub crontab executable that triggers the read error for the failed user' do + stub_crontab_bin_dir = agent.tmpdir('stub_crontab_bin_dir') + stub_crontab_exe = "#{stub_crontab_bin_dir}/crontab" + + # On Linux and OSX, we read a user's crontab by running crontab -u , + # where the crontab command is run as root. However on AIX/Solaris, we read a + # user's crontab by running the crontab command as that user. Thus our mock + # crontab executable needs to check if we're reading our failed user's crontab + # (Linux and OSX) OR running crontab as our failed user (AIX and Solaris) before + # triggering the FileReadError + stub_crontab_exe_script = <<-SCRIPT +#!/usr/bin/env bash + if [[ "$@" =~ #{failed_username} || "`id`" =~ #{failed_username} ]]; then + echo "Mocking a FileReadError for the #{failed_username} user's crontab!" + exit 1 +fi + #{crontab_exe} $@ +SCRIPT + + create_remote_file(agent, stub_crontab_exe, stub_crontab_exe_script) + on(agent, "chmod 777 #{stub_crontab_bin_dir}") + on(agent, "chmod 777 #{stub_crontab_exe}") + end + + path_env_var = nil + step 'Get the value of the PATH environment variable' do + path_env_var = on(agent, 'echo $PATH').stdout.chomp + end + + puppet_result = nil + step 'Add some cron entries with Puppet' do + # We delete our mock crontab executable here to ensure that Cron[second_entry]'s + # evaluation fails because of the FileReadError raised in the prefetch + # step. Otherwise, Cron[second_entry]'s evaluation will fail at the write step + # because Puppet would still be invoking our mock crontab executable, which would + # pass the test on an agent that swallows FileReadErrors in the cron provider's + # prefetch step. + manifest = [ + cron_manifest('first_entry', command: 'ls', user: username), + file_manifest(stub_crontab_exe, ensure: :absent), + cron_manifest('second_entry', command: 'ls', user: failed_username), + ].join("\n\n") + manifest_file = agent.tmpfile('crontab_overwrite_manifest') + create_remote_file(agent, manifest_file, manifest) + + # We need to run a script here instead of a command because: + # * We need to cd into a directory that our user can access. Otherwise, bash will + # fail to execute stub_crontab_exe on AIX and Solaris because we run crontab + # as the given user, and the given user does not have access to Puppet's cwd. + # + # * We also need to pass-in our PATH to Puppet since it contains stub_crontab_bin_dir. + apply_crontab_overwrite_manifest = agent.tmpfile('apply_crontab_overwrite_manifest') + script = <<-SCRIPT + #!/usr/bin/env bash + cd #{stub_crontab_bin_dir} && puppet apply #{manifest_file} + SCRIPT + create_remote_file(agent, apply_crontab_overwrite_manifest, script) + on(agent, "chmod a+x #{apply_crontab_overwrite_manifest}") + + puppet_result = on(agent, "bash #{apply_crontab_overwrite_manifest}", environment: { PATH: "#{stub_crontab_bin_dir}:#{path_env_var}" }) + end + + step 'Verify that Puppet fails a Cron resource associated with an unreadable crontab file' do + assert_match(%r{Cron.*second_entry}, puppet_result.stderr, 'Puppet does not fail a Cron resource associated with an unreadable crontab file') + end + + step 'Verify that Puppet does not fail a Cron resource associated with a readable crontab file' do + assert_no_match(%r{Cron.*first_entry}, puppet_result.stderr, 'Puppet fails a Cron resource associated with a readable crontab file') + end + + step 'Verify that Puppet successfully evaluates a Cron resource associated with a readable crontab file' do + assert_match(%r{Cron.*first_entry}, puppet_result.stdout, 'Puppet fails to evaluate a Cron resource associated with a readable crontab file') + end + + step 'Verify that Puppet did update the readable crontab file with the Cron resource' do + assert_matching_arrays(['* * * * * ls'], crontab_entries_of(agent, username), 'Puppet fails to update a readable crontab file with the specified Cron entry') + end + end + end +end -- cgit v1.2.3 From 071e84c6974d3f783abc8b5a0ded66684f6ac65f Mon Sep 17 00:00:00 2001 From: Enis Inan Date: Wed, 9 Jan 2019 09:53:37 -0800 Subject: (maint) Simplify the confine in the unauthorized users acceptance test --- ...orized_users_crontab_if_puppet_authorizes_them_spec.rb | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) (limited to 'spec/acceptance/tests') diff --git a/spec/acceptance/tests/resource/cron/should_write_an_originally_unauthorized_users_crontab_if_puppet_authorizes_them_spec.rb b/spec/acceptance/tests/resource/cron/should_write_an_originally_unauthorized_users_crontab_if_puppet_authorizes_them_spec.rb index 964cc17..834cac7 100644 --- a/spec/acceptance/tests/resource/cron/should_write_an_originally_unauthorized_users_crontab_if_puppet_authorizes_them_spec.rb +++ b/spec/acceptance/tests/resource/cron/should_write_an_originally_unauthorized_users_crontab_if_puppet_authorizes_them_spec.rb @@ -1,17 +1,6 @@ require 'spec_helper_acceptance' RSpec.context 'when Puppet authorizes a previously unauthorized user to use crontab' do - class << self - alias_method :orig_compatible_agents, :compatible_agents - - # This test is confined to AIX and Solaris agents only. - def compatible_agents - orig_compatible_agents.select do |agent| - agent['platform'].include?('aix') || agent['platform'].include?('solaris') - end - end - end - let(:username) { "pl#{rand(999_999).to_i}" } let(:unauthorized_username) { "pl#{rand(999_999).to_i}" } @@ -70,7 +59,9 @@ RSpec.context 'when Puppet authorizes a previously unauthorized user to use cron end compatible_agents.each do |agent| - it "should write that user's crontab on #{agent}" do + is_aix_or_solaris_agent = agent['platform'].include?('aix') || agent['platform'].include?('solaris') + + it "should write that user's crontab on #{agent}", if: is_aix_or_solaris_agent do step 'Add the unauthorized user to the cron.deny file' do on(agent, "echo #{unauthorized_username} >> #{cron_deny_path[agent]}") end -- cgit v1.2.3 From c0068fc6535289f7828ba42ad8610914a57604f1 Mon Sep 17 00:00:00 2001 From: Enis Inan Date: Wed, 9 Jan 2019 10:27:10 -0800 Subject: (MODULES-7789) Don't run new tests on older agents We only want to run these tests on agents that shipped with PUP-9217's changes. These are any agents newer than 5.5.8 in the 5.y release stream, and any agents newer than 6.0.4 in the 6.y release stream. --- ...erwrite_crontab_file_on_file_read_error_spec.rb | 33 +++++++++++++++++----- ...associated_resources_on_file_read_error_spec.rb | 29 ++++++++++++++++--- 2 files changed, 51 insertions(+), 11 deletions(-) (limited to 'spec/acceptance/tests') diff --git a/spec/acceptance/tests/resource/cron/should_not_overwrite_crontab_file_on_file_read_error_spec.rb b/spec/acceptance/tests/resource/cron/should_not_overwrite_crontab_file_on_file_read_error_spec.rb index 08e8bb8..6a5b2c6 100644 --- a/spec/acceptance/tests/resource/cron/should_not_overwrite_crontab_file_on_file_read_error_spec.rb +++ b/spec/acceptance/tests/resource/cron/should_not_overwrite_crontab_file_on_file_read_error_spec.rb @@ -1,18 +1,31 @@ require 'spec_helper_acceptance' RSpec.context 'when Puppet cannot read a crontab file' do + # This test only makes sense for agents that shipped with PUP-9217's + # changes, so we do not want to run it on older agents. + def older_agent?(agent) + puppet_version = Gem::Version.new(on(agent, puppet('--version')).stdout.chomp) + minimum_puppet_version = if puppet_version < Gem::Version.new('6.0.0') + Gem::Version.new('5.5.9') + else + Gem::Version.new('6.0.5') + end + + puppet_version < minimum_puppet_version + end + let(:username) { "pl#{rand(999_999).to_i}" } let(:crontab_contents) { '6 6 6 6 6 /usr/bin/true' } before(:each) do - step 'Create the user on the agents' do - compatible_agents.each do |agent| + compatible_agents.each do |agent| + next if older_agent?(agent) + + step "Create the user on #{agent}" do user_present(agent, username) end - end - step "Set the user's crontab" do - compatible_agents.each do |agent| + step "Set the user's crontab on #{agent}" do run_cron_on(agent, :add, username, crontab_contents) assert_matching_arrays([crontab_contents], crontab_entries_of(agent, username), "Could not set the user's crontab for testing") end @@ -20,8 +33,10 @@ RSpec.context 'when Puppet cannot read a crontab file' do end after(:each) do - step 'Teardown -- Erase the user on the agents' do - compatible_agents.each do |agent| + compatible_agents.each do |agent| + next if older_agent?(agent) + + step "Teardown -- Erase the user on #{agent}" do run_cron_on(agent, :remove, username) user_absent(agent, username) end @@ -30,6 +45,10 @@ RSpec.context 'when Puppet cannot read a crontab file' do compatible_agents.each do |agent| it "should not overwrite it on #{agent}" do + if older_agent?(agent) + skip('Skipping this test since we are on an older agent that does not have the PUP-9217 changes') + end + crontab_exe = nil step 'Find the crontab executable' do crontab_exe = on(agent, 'which crontab').stdout.chomp diff --git a/spec/acceptance/tests/resource/cron/should_only_fail_associated_resources_on_file_read_error_spec.rb b/spec/acceptance/tests/resource/cron/should_only_fail_associated_resources_on_file_read_error_spec.rb index 3e06d0c..1846523 100644 --- a/spec/acceptance/tests/resource/cron/should_only_fail_associated_resources_on_file_read_error_spec.rb +++ b/spec/acceptance/tests/resource/cron/should_only_fail_associated_resources_on_file_read_error_spec.rb @@ -1,12 +1,27 @@ require 'spec_helper_acceptance' RSpec.context 'when Puppet cannot read a crontab file' do + # This test only makes sense for agents that shipped with PUP-9217's + # changes, so we do not want to run it on older agents. + def older_agent?(agent) + puppet_version = Gem::Version.new(on(agent, puppet('--version')).stdout.chomp) + minimum_puppet_version = if puppet_version < Gem::Version.new('6.0.0') + Gem::Version.new('5.5.9') + else + Gem::Version.new('6.0.5') + end + + puppet_version < minimum_puppet_version + end + let(:username) { "pl#{rand(999_999).to_i}" } let(:failed_username) { "pl#{rand(999_999).to_i}" } before(:each) do - step 'Create the users' do - compatible_agents.each do |agent| + compatible_agents.each do |agent| + next if older_agent?(agent) + + step "Create the users on #{agent}" do user_present(agent, username) user_present(agent, failed_username) end @@ -14,8 +29,10 @@ RSpec.context 'when Puppet cannot read a crontab file' do end after(:each) do - step 'Teardown -- Erase the users' do - compatible_agents.each do |agent| + compatible_agents.each do |agent| + next if older_agent?(agent) + + step "Teardown -- Erase the users on #{agent}" do run_cron_on(agent, :remove, username) user_absent(agent, username) @@ -26,6 +43,10 @@ RSpec.context 'when Puppet cannot read a crontab file' do compatible_agents.each do |agent| it "should only fail the associated resources on #{agent}" do + if older_agent?(agent) + skip('Skipping this test since we are on an older agent that does not have the PUP-9217 changes') + end + crontab_exe = nil step 'Find the crontab executable' do crontab_exe = on(agent, 'which crontab').stdout.chomp -- cgit v1.2.3