CHEF-2745 Fix issue with generic environment variables that match int…

…ernals
aglarond · Apr 11, 2012 · 1e8ada8 · 1e8ada8
1 parent 2d0d6f2
commit 1e8ada8
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 13 deletions.
diff --git a/chef/lib/chef/provider/cron.rb b/chef/lib/chef/provider/cron.rb
@@ -26,8 +26,9 @@ class Cron < Chef::Provider
       include Chef::Mixin::Command
 
       CRON_PATTERN = /\A([-0-9*,\/]+)\s([-0-9*,\/]+)\s([-0-9*,\/]+)\s([-0-9*,\/]+)\s([-0-9*,\/]+)\s(.*)/
+      ENV_PATTERN = /\A(\S+)=(\S*)/
+
       CRON_ATTRIBUTES = [:minute, :hour, :day, :month, :weekday, :command, :mailto, :path, :shell, :home, :environment]
-      ENV_PATTERN = /\A[A-Z]+=/
 
       def initialize(new_resource, run_context)
         super(new_resource, run_context)
@@ -49,7 +50,7 @@ def load_current_resource
               cron_found = true
               @cron_exists = true
               next
-            when /^(\S*)=(\S*)/
+            when ENV_PATTERN
               set_environment_var($1, $2) if cron_found
               next
             when CRON_PATTERN
@@ -173,9 +174,8 @@ def action_delete
       private
 
       def set_environment_var(attr_name, attr_value)
-        method_name = attr_name.downcase.to_sym
-        if CRON_ATTRIBUTES.include?(method_name)
-          @current_resource.send(method_name, attr_value)
+        if %w(MAILTO PATH SHELL HOME).include?(attr_name)
+          @current_resource.send(attr_name.downcase.to_sym, attr_value)
         else
           @current_resource.environment(@current_resource.environment.merge(attr_name => attr_value))
         end

diff --git a/chef/spec/unit/provider/cron_spec.rb b/chef/spec/unit/provider/cron_spec.rb
@@ -140,11 +140,6 @@
         cron.command.should == '/bin/true param1 param2'
       end
 
-      it "should report the match" do
-        Chef::Log.should_receive(:debug).with("Found cron '#{@new_resource.name}'")
-        @provider.load_current_resource
-      end
-
       it "should parse and load generic and standard environment variables from cron entry" do
         @provider.stub!(:read_crontab).and_return(<<-CRONTAB)
 # Chef Name: cronhole some stuff
@@ -153,10 +148,31 @@
 FLAG=1
 * 5 * * * /bin/true
 CRONTAB
-        resource = @provider.load_current_resource
+        cron = @provider.load_current_resource
+
+        cron.mailto.should == "warn@example.com"
+        cron.environment.should == {"TEST" => "lol", "FLAG" => "1"}
+      end
+
+      it "should not break with variabels that match the cron resource internals" do
+        @provider.stub!(:read_crontab).and_return(<<-CRONTAB)
+# Chef Name: cronhole some stuff
+MINUTE=40
+HOUR=midnight
+TEST=lol
+ENVIRONMENT=production
+* 5 * * * /bin/true
+CRONTAB
+        cron = @provider.load_current_resource
 
-        resource.mailto.should == "warn@example.com"
-        resource.environment.should == {"TEST" => "lol", "FLAG" => "1"}
+        cron.minute.should == '*'
+        cron.hour.should == '5'
+        cron.environment.should == {"MINUTE" => "40", "HOUR" => "midnight", "TEST" => "lol", "ENVIRONMENT" => "production"}
+      end
+
+      it "should report the match" do
+        Chef::Log.should_receive(:debug).with("Found cron '#{@new_resource.name}'")
+        @provider.load_current_resource
       end
     end