Differentiate between the default and the user set guard_attributes in order to be able to warn users correctly when they configure a guard_attribute but use a ruby block in the guard.

3 files changed, 35 insertions, 27 deletions

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 70abfbcdb0..55de8f059c 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -229,6 +229,8 @@ F
 
     attr_reader :elapsed_time
 
+    attr_reader :default_guard_interpreter
+
     # Each notify entry is a resource/action pair, modeled as an
     # Struct with a #resource and #action member
 
@@ -250,7 +252,13 @@ F
       @not_if = []
       @only_if = []
       @source_line = nil
-      @guard_interpreter = :default
+      # We would like to raise an error when the user gives us a guard
+      # interpreter and a ruby_block to the guard. In order to achieve this
+      # we need to understand when the user overrides the default guard
+      # interpreter. Therefore we store the default separately in a different
+      # attribute.
+      @guard_interpreter = nil
+      @default_guard_interpreter = :default
       @elapsed_time = 0
       @sensitive = false
     end
@@ -410,11 +418,15 @@ F
     end
 
     def guard_interpreter(arg=nil)
-      set_or_return(
-        :guard_interpreter,
-        arg,
-        :kind_of => Symbol
-      )
+      if arg.nil?
+        @guard_interpreter || @default_guard_interpreter
+      else
+        set_or_return(
+          :guard_interpreter,
+          arg,
+          :kind_of => Symbol
+        )
+      end
     end
 
     # Sets up a notification from this resource to the resource specified by +resource_spec+.
diff --git a/lib/chef/resource/conditional.rb b/lib/chef/resource/conditional.rb
index 6a987d3086..8960a4d57f 100644
--- a/lib/chef/resource/conditional.rb
+++ b/lib/chef/resource/conditional.rb
@@ -54,29 +54,25 @@ class Chef
       end
 
       def configure
-        if @block_given
-          # If a block is given, we will not interpret the block with a guard interpreter.
+        case @command
+        when String
+          @guard_interpreter = new_guard_interpreter(@parent_resource, @command, @command_opts, &@block)
+          @block = nil
+        when nil
+          # We should have a block if we get here
+          # Check to see if the user set the guard_interpreter on the parent resource. Note that
+          # this error will not be raised when using the default_guard_interpreter
+          if @parent_resource.guard_interpreter != @parent_resource.default_guard_interpreter
+            msg = "#{@parent_resource.name} was given a guard_interpreter of #{@parent_resource.guard_interpreter}, "
+            msg << "but not given a command as a string. guard_interpreter does not support blocks (because they just contain ruby)."
+            raise ArgumentError, msg
+          end
+
           @guard_interpreter = nil
           @command, @command_opts = nil, nil
         else
-          case @command
-          when String
-            @guard_interpreter = new_guard_interpreter(@parent_resource, @command, @command_opts, &@block)
-            @block = nil
-          when nil
-            # We should have a block if we get here
-            if @parent_resource.guard_interpreter != :default
-              msg = "#{@parent_resource.name} was given a guard_interpreter of #{@parent_resource.guard_interpreter}, "
-              msg << "but not given a command as a string. guard_interpreter does not support blocks (because they just contain ruby)."
-              raise ArgumentError, msg
-            end
-
-            @guard_interpreter = nil
-            @command, @command_opts = nil, nil
-          else
-            # command was passed, but it wasn't a String
-            raise ArgumentError, "Invalid only_if/not_if command, expected a string: #{command.inspect} (#{command.class})"
-          end
+          # command was passed, but it wasn't a String
+          raise ArgumentError, "Invalid only_if/not_if command, expected a string: #{command.inspect} (#{command.class})"
         end
       end
 
diff --git a/lib/chef/resource/execute.rb b/lib/chef/resource/execute.rb
index e0a1bbb8c0..ae118b1c9e 100644
--- a/lib/chef/resource/execute.rb
+++ b/lib/chef/resource/execute.rb
@@ -40,7 +40,7 @@ class Chef
         @user = nil
         @allowed_actions.push(:run)
         @umask = nil
-        @guard_interpreter = :execute
+        @default_guard_interpreter = :execute
       end
 
       def umask(arg=nil)