Allow values to be set to nil (override defaults) if user allows it

2 files changed, 58 insertions, 53 deletions

diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb
index db25ecd847..2d382d8381 100644
--- a/lib/chef/mixin/params_validate.rb
+++ b/lib/chef/mixin/params_validate.rb
@@ -16,6 +16,8 @@
 # limitations under the License.
 
 class Chef
+  NOT_PASSED = Object.new
+
   class DelayedEvaluator < Proc
   end
   module Mixin
@@ -81,34 +83,46 @@ class Chef
         DelayedEvaluator.new(&block)
       end
 
-      def set_or_return(symbol, arg, validation)
+      def set_or_return(symbol, value, validation)
         iv_symbol = "@#{symbol.to_s}".to_sym
-        if arg.nil? && self.instance_variable_defined?(iv_symbol) == true
-          ivar = self.instance_variable_get(iv_symbol)
-          if(ivar.is_a?(DelayedEvaluator))
-            validate({ symbol => ivar.call }, { symbol => validation })[symbol]
+
+        # If the user passed NOT_PASSED, or passed nil, then this is a get.
+        case value
+        when NOT_PASSED
+          is_get = true
+          value = nil
+        when nil
+          is_get = true unless explicitly_allows_nil?(symbol, validation)
+        end
+
+        if self.instance_variable_defined?(iv_symbol) && is_get
+          value = self.instance_variable_get(iv_symbol)
+          if value.is_a?(DelayedEvaluator)
+            validate({ symbol => value.call }, { symbol => validation })[symbol]
           else
-            ivar
+            value
           end
         else
-          if(arg.is_a?(DelayedEvaluator))
-            val = arg
-          else
-            val = validate({ symbol => arg }, { symbol => validation })[symbol]
+          if !value.is_a?(DelayedEvaluator)
+            value = validate({ symbol => value }, { symbol => validation })[symbol]
 
             # Handle the case where the "default" was a DelayedEvaluator. In
             # this case, the block yields an optional parameter of +self+,
             # which is the equivalent of "new_resource"
-            if val.is_a?(DelayedEvaluator)
-              val = val.call(self)
+            if value.is_a?(DelayedEvaluator)
+              value = value.call(self)
             end
           end
-          self.instance_variable_set(iv_symbol, val)
+          self.instance_variable_set(iv_symbol, value)
         end
       end
 
       private
 
+      def explicitly_allows_nil?(key, validation)
+        validation.has_key?(:is) && _pv_is({ key => nil }, key, validation[:is], raise_error: false)
+      end
+
       # Return the value of a parameter, or nil if it doesn't exist.
       def _pv_opts_lookup(opts, key)
         if opts.has_key?(key.to_s)
@@ -135,14 +149,11 @@ class Chef
       def _pv_equal_to(opts, key, to_be)
         value = _pv_opts_lookup(opts, key)
         unless value.nil?
-          passes = false
           to_be = Array(to_be)
           to_be.each do |tb|
-            passes = true if value == tb
-          end
-          unless passes
-            raise Exceptions::ValidationFailed, "Option #{key} must be equal to one of: #{to_be.join(", ")}!  You passed #{value.inspect}."
+            return true if value == tb
           end
+          raise Exceptions::ValidationFailed, "Option #{key} must be equal to one of: #{to_be.join(", ")}!  You passed #{value.inspect}."
         end
       end
 
@@ -150,14 +161,11 @@ class Chef
       def _pv_kind_of(opts, key, to_be)
         value = _pv_opts_lookup(opts, key)
         unless value.nil?
-          passes = false
           to_be = Array(to_be)
           to_be.each do |tb|
-            passes = true if value.kind_of?(tb)
-          end
-          unless passes
-            raise Exceptions::ValidationFailed, "Option #{key} must be a kind of #{to_be}!  You passed #{value.inspect}."
+            return true if value.kind_of?(tb)
           end
+          raise Exceptions::ValidationFailed, "Option #{key} must be a kind of #{to_be}!  You passed #{value.inspect}."
         end
       end
 
@@ -182,12 +190,14 @@ class Chef
       # both :cannot_be => [ :blank, :nil ]
       def _pv_cannot_be(opts, key, predicate_method_base_name)
         value = _pv_opts_lookup(opts, key)
-        Array(predicate_method_base_name).each do |method_name|
-          predicate_method = :"#{method_name}?"
+        if !value.nil?
+          Array(predicate_method_base_name).each do |method_name|
+            predicate_method = :"#{method_name}?"
 
-          if value.respond_to?(predicate_method)
-            if value.send(predicate_method)
-              raise Exceptions::ValidationFailed, "Option #{key} cannot be #{predicate_method_base_name}"
+            if value.respond_to?(predicate_method)
+              if value.send(predicate_method)
+                raise Exceptions::ValidationFailed, "Option #{key} cannot be #{predicate_method_base_name}"
+              end
             end
           end
         end
@@ -205,17 +215,10 @@ class Chef
       def _pv_regex(opts, key, regex)
         value = _pv_opts_lookup(opts, key)
         if !value.nil?
-          passes = false
           Array(regex).each do |r|
-            if value != nil
-              if r.match(value.to_s)
-                passes = true
-              end
-            end
-          end
-          unless passes
-            raise Exceptions::ValidationFailed, "Option #{key}'s value #{value} does not match regular expression #{regex.inspect}"
+            return true if r.match(value.to_s)
           end
+          raise Exceptions::ValidationFailed, "Option #{key}'s value #{value} does not match regular expression #{regex.inspect}"
         end
       end
 
@@ -223,7 +226,7 @@ class Chef
       def _pv_callbacks(opts, key, callbacks)
         raise ArgumentError, "Callback list must be a hash!" unless callbacks.kind_of?(Hash)
         value = _pv_opts_lookup(opts, key)
-        if value != nil
+        if !value.nil?
           callbacks.each do |message, zeproc|
             if zeproc.call(value) != true
               raise Exceptions::ValidationFailed, "Option #{key}'s value #{value} #{message}!"
@@ -243,18 +246,22 @@ class Chef
       alias :_pv_name_attribute :_pv_name_property
 
       # Compare the way "case" would (i.e. `===`)
-      def _pv_is(opts, key, to_be)
+      def _pv_is(opts, key, to_be, raise_error: true)
         value = _pv_opts_lookup(opts, key)
         to_be = [ to_be ].flatten(1)
         to_be.each do |tb|
           if tb.is_a?(Proc)
-            return if instance_exec(value, &tb)
+            return true if instance_exec(value, &tb)
           else
-            return if tb === value
+            return true if tb === value
           end
         end
 
-        raise Exceptions::ValidationFailed, "Option #{key} must be one of: #{to_be.join(", ")}!  You passed #{value.inspect}."
+        if raise_error
+          raise Exceptions::ValidationFailed, "Option #{key} must be one of: #{to_be.join(", ")}!  You passed #{value.inspect}."
+        else
+          false
+        end
       end
     end
   end
diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 44247ca9f0..9496656705 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -58,8 +58,6 @@ class Chef
     include Chef::Mixin::ShellOut
     include Chef::Mixin::PowershellOut
 
-    NULL_ARG = Object.new
-
     #
     # The node the current Chef run is using.
     #
@@ -773,10 +771,10 @@ class Chef
     # @example With type and options
     #   property :x, String, default: 'hi'
     #
-    def self.property(name, type=NULL_ARG, **options)
+    def self.property(name, type=NOT_PASSED, **options)
       name = name.to_sym
 
-      if type != NULL_ARG
+      if type != NOT_PASSED
         if options[:is]
           options[:is] = ([ type ] + [ options[:is] ]).flatten(1)
         else
@@ -784,7 +782,7 @@ class Chef
         end
       end
 
-      define_method(name) do |value=nil|
+      define_method(name) do |value=NOT_PASSED|
         set_or_return(name, value, options)
       end
       define_method("#{name}=") do |value|
@@ -868,8 +866,8 @@ class Chef
     #   have.
     #
     attr_accessor :allowed_actions
-    def allowed_actions(value=NULL_ARG)
-      if value != NULL_ARG
+    def allowed_actions(value=NOT_PASSED)
+      if value != NOT_PASSED
         self.allowed_actions = value
       end
       @allowed_actions
@@ -1003,9 +1001,9 @@ class Chef
     #
     # @return [Symbol] The name of this resource type (e.g. `:execute`).
     #
-    def self.resource_name(name=NULL_ARG)
+    def self.resource_name(name=NOT_PASSED)
       # Setter
-      if name != NULL_ARG
+      if name != NOT_PASSED
         remove_canonical_dsl
 
         # Set the resource_name and call provides
@@ -1095,8 +1093,8 @@ class Chef
     #
     # @return [Symbol,Array<Symbol>] The default actions for the resource.
     #
-    def self.default_action(action_name=NULL_ARG)
-      unless action_name.equal?(NULL_ARG)
+    def self.default_action(action_name=NOT_PASSED)
+      unless action_name.equal?(NOT_PASSED)
         if action_name.is_a?(Array)
           @default_action = action_name.map { |arg| arg.to_sym }
         else