Merge pull request #5127 from chef/tm/empty_cron_attributes

Support setting an empty string for cron attrs

3 files changed, 25 insertions, 18 deletions

diff --git a/lib/chef/provider/cron.rb b/lib/chef/provider/cron.rb
index 36b67ab6a5..c7487cf42f 100644
--- a/lib/chef/provider/cron.rb
+++ b/lib/chef/provider/cron.rb
@@ -237,7 +237,7 @@ class Chef
         newcron = ""
         newcron << "# Chef Name: #{new_resource.name}\n"
         [ :mailto, :path, :shell, :home ].each do |v|
-          newcron << "#{v.to_s.upcase}=#{@new_resource.send(v)}\n" if @new_resource.send(v)
+          newcron << "#{v.to_s.upcase}=\"#{@new_resource.send(v)}\"\n" if @new_resource.send(v)
         end
         @new_resource.environment.each do |name, value|
           newcron << "#{name}=#{value}\n"
diff --git a/spec/functional/resource/cron_spec.rb b/spec/functional/resource/cron_spec.rb
index 3380eccb0d..f5948191c5 100644
--- a/spec/functional/resource/cron_spec.rb
+++ b/spec/functional/resource/cron_spec.rb
@@ -120,7 +120,7 @@ describe Chef::Resource::Cron, :requires_root, :unix_only do
       return if %w{aix solaris}.include?(ohai[:platform])
       # Test if the attribute exists on newly created cron
       cron_should_exists(cron_name, "")
-      expect(shell_out("crontab -l -u #{new_resource.user} | grep \"#{attribute.upcase}=#{value}\"").exitstatus).to eq(0)
+      expect(shell_out("crontab -l -u #{new_resource.user} | grep '#{attribute.upcase}=\"#{value}\"'").exitstatus).to eq(0)
     end
 
     after do
@@ -146,6 +146,13 @@ describe Chef::Resource::Cron, :requires_root, :unix_only do
       new_resource.home "/home/opscode"
       create_and_validate_with_attribute(new_resource, "home", "/home/opscode")
     end
+
+    %i{ home mailto path shell }.each do |attr|
+      it "supports an empty string for #{attr} attribute" do
+        new_resource.send(attr, "")
+        create_and_validate_with_attribute(new_resource, attr.to_s, "")
+      end
+    end
   end
 
   describe "negative tests for create action" do
diff --git a/spec/unit/provider/cron_spec.rb b/spec/unit/provider/cron_spec.rb
index 010b1b09f3..9e849743e7 100644
--- a/spec/unit/provider/cron_spec.rb
+++ b/spec/unit/provider/cron_spec.rb
@@ -462,10 +462,10 @@ CRONTAB
         @new_resource.environment "TEST" => "LOL"
         expect(@provider).to receive(:write_crontab).with(<<-ENDCRON)
 # Chef Name: cronhole some stuff
-MAILTO=foo@example.com
-PATH=/usr/bin:/my/custom/path
-SHELL=/bin/foosh
-HOME=/home/foo
+MAILTO="foo@example.com"
+PATH="/usr/bin:/my/custom/path"
+SHELL="/bin/foosh"
+HOME="/home/foo"
 TEST=LOL
 30 * * * * /bin/true
         ENDCRON
@@ -524,10 +524,10 @@ TEST=LOL
 
 # Another comment
 # Chef Name: cronhole some stuff
-MAILTO=foo@example.com
-PATH=/usr/bin:/my/custom/path
-SHELL=/bin/foosh
-HOME=/home/foo
+MAILTO="foo@example.com"
+PATH="/usr/bin:/my/custom/path"
+SHELL="/bin/foosh"
+HOME="/home/foo"
 TEST=LOL
 30 * * * * /bin/true
         ENDCRON
@@ -585,10 +585,10 @@ TEST=LOL
 0 2 * * * /some/other/command
 
 # Chef Name: cronhole some stuff
-MAILTO=foo@example.com
-PATH=/usr/bin:/my/custom/path
-SHELL=/bin/foosh
-HOME=/home/foo
+MAILTO="foo@example.com"
+PATH="/usr/bin:/my/custom/path"
+SHELL="/bin/foosh"
+HOME="/home/foo"
 TEST=LOL
 30 * * * * /bin/true
 # Chef Name: something else
@@ -679,10 +679,10 @@ HOME=/home/foo
 0 2 * * * /some/other/command
 
 # Chef Name: cronhole some stuff
-MAILTO=foo@example.com
-PATH=/usr/bin:/my/custom/path
-SHELL=/bin/foosh
-HOME=/home/foo
+MAILTO="foo@example.com"
+PATH="/usr/bin:/my/custom/path"
+SHELL="/bin/foosh"
+HOME="/home/foo"
 30 * * * * /bin/true
 
 # Chef Name: something else