summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatthew Newell <18470637+wheatevo@users.noreply.github.com>2021-10-14 16:12:43 -0500
committerTim Smith <tsmith84@gmail.com>2021-11-09 16:46:25 -0800
commit5c7a67ac2edbb11976ff972e15b2e551ad98d51c (patch)
tree52b6eb14251d0fa7c21398c8a7162e3aba493aac
parent4bf644037f2409b5fcf4e1aedb1736a2b26b1b57 (diff)
downloadchef-cron_16.tar.gz
Fix cron resource commented job handlingcron_16
This change fixes an edge case failure that may occur when processing a cron resource where the following conditions are true: * A `# Chef Name` comment exists for the job * The following line is commented out * The new cron resource uses default values for the minute, hour, day, month, weekday, and time properties This causes the following error to be raised within the `Chef::Provider::Cron#cron_different?` method: ``` Chef::Exceptions::ValidationFailed ---------------------------------- command is a required property ``` The new guard prevents `cron_different?` from being called when the current resource does not have a set command property. Since the new resource requires the command property to be set to be valid, we can assume it is different from the current cron resource if the command is unset. Signed-off-by: Matthew Newell <18470637+wheatevo@users.noreply.github.com>
-rw-r--r--lib/chef/provider/cron.rb7
-rw-r--r--spec/unit/provider/cron_spec.rb45
2 files changed, 50 insertions, 2 deletions
diff --git a/lib/chef/provider/cron.rb b/lib/chef/provider/cron.rb
index 7d37f34b1a..694f7ba4b8 100644
--- a/lib/chef/provider/cron.rb
+++ b/lib/chef/provider/cron.rb
@@ -100,8 +100,11 @@ class Chef
newcron = get_crontab_entry
if @cron_exists
- unless cron_different?
- logger.trace("Skipping existing cron entry '#{new_resource.name}'")
+ # Only compare the crontab if the current resource has a set command.
+ # This may not be set in cases where the Chef comment exists but the
+ # crontab command was commented out.
+ if current_resource.property_is_set?(:command) && !cron_different?
+ logger.debug("#{new_resource}: Skipping existing cron entry")
return
end
read_crontab.each_line do |line|
diff --git a/spec/unit/provider/cron_spec.rb b/spec/unit/provider/cron_spec.rb
index 76f170312e..ac92f45cf8 100644
--- a/spec/unit/provider/cron_spec.rb
+++ b/spec/unit/provider/cron_spec.rb
@@ -392,6 +392,27 @@ describe Chef::Provider::Cron do
expect(cron.command).to eq("/bin/true")
end
end
+
+ context "with a matching entry with a commented crontab line" do
+ it "should set cron_exists" do
+ allow(@provider).to receive(:read_crontab).and_return(<<~CRONTAB)
+ 0 2 * * * /some/other/command
+
+ # Chef Name: cronhole some stuff
+ #* * * * * /bin/true
+ CRONTAB
+ cron = @provider.load_current_resource
+ expect(@provider.cron_exists).to eq(true)
+ expect(@provider.cron_empty).to eq(false)
+ expect(cron.minute).to eq("*")
+ expect(cron.hour).to eq("*")
+ expect(cron.day).to eq("*")
+ expect(cron.month).to eq("*")
+ expect(cron.weekday).to eq("*")
+ expect(cron.time).to eq(nil)
+ expect(cron.property_is_set?(:command)).to eq(false)
+ end
+ end
end
describe "cron_different?" do
@@ -691,6 +712,30 @@ describe Chef::Provider::Cron do
end
end
+ context "when there is a crontab with a matching section with a commented crontab line in it" do
+ before :each do
+ @provider.cron_exists = true
+ end
+
+ it "should add the crontab to the entry" do
+ allow(@provider).to receive(:read_crontab).and_return(<<~CRONTAB)
+ 0 2 * * * /some/other/command
+
+ # Chef Name: cronhole some stuff
+ # * * * * * /bin/true
+ CRONTAB
+ expect(@provider).to receive(:write_crontab).with(<<~ENDCRON)
+ 0 2 * * * /some/other/command
+
+ # Chef Name: cronhole some stuff
+ * * * * * /bin/true
+ # * * * * * /bin/true
+ ENDCRON
+ @new_resource.minute "*"
+ @provider.run_action(:create)
+ end
+ end
+
context "when there is a crontab with a matching and identical section" do
context "when environment variable is not used" do
before :each do