diff options
Diffstat (limited to 'lib')
| -rw-r--r-- | lib/chef/exceptions.rb | 1 | ||||
| -rw-r--r-- | lib/chef/mixin/params_validate.rb | 12 | ||||
| -rw-r--r-- | lib/chef/property.rb | 129 | ||||
| -rw-r--r-- | lib/chef/resource/chef_gem.rb | 4 | ||||
| -rw-r--r-- | lib/chef/resource/file.rb | 2 |
5 files changed, 110 insertions, 38 deletions
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb index f9a717471c..08d916064f 100644 --- a/lib/chef/exceptions.rb +++ b/lib/chef/exceptions.rb @@ -71,6 +71,7 @@ class Chef class RoleNotFound < RuntimeError; end class DuplicateRole < RuntimeError; end class ValidationFailed < ArgumentError; end + class CannotValidateStaticallyError < ArgumentError; end class InvalidPrivateKey < ArgumentError; end class MissingKeyAttribute < ArgumentError; end class KeyCommandInputError < ArgumentError; end diff --git a/lib/chef/mixin/params_validate.rb b/lib/chef/mixin/params_validate.rb index 841cf9e6a2..3585def853 100644 --- a/lib/chef/mixin/params_validate.rb +++ b/lib/chef/mixin/params_validate.rb @@ -22,7 +22,6 @@ require "chef/delayed_evaluator" class Chef module Mixin module ParamsValidate - # Takes a hash of options, along with a map to validate them. Returns the original # options hash, plus any changes that might have been made (through things like setting # default values in the validation map) @@ -333,6 +332,7 @@ class Chef def _pv_name_property(opts, key, is_name_property=true) if is_name_property if opts[key].nil? + raise CannotValidateStaticallyError, "name_property cannot be evaluated without a resource." if self == Chef::Mixin::ParamsValidate opts[key] = self.instance_variable_get(:"@name") end end @@ -403,6 +403,7 @@ class Chef to_be.each do |tb| case tb when Proc + raise CannotValidateStaticallyError, "is: proc { } must be evaluated once for each resource" if self == Chef::Mixin::ParamsValidate return true if instance_exec(value, &tb) when Property validate(opts, { key => tb.validation_options }) @@ -436,12 +437,21 @@ class Chef # def _pv_coerce(opts, key, coercer) if opts.has_key?(key.to_s) + raise CannotValidateStaticallyError, "coerce must be evaluated for each resource." if self == Chef::Mixin::ParamsValidate opts[key.to_s] = instance_exec(opts[key], &coercer) elsif opts.has_key?(key.to_sym) + raise CannotValidateStaticallyError, "coerce must be evaluated for each resource." if self == Chef::Mixin::ParamsValidate opts[key.to_sym] = instance_exec(opts[key], &coercer) end end + # We allow Chef::Mixin::ParamsValidate.validate(), but we will raise an + # error if you try to do anything requiring there to be an actual resource. + # This way, you can statically validate things if you have constant validation + # (which is the norm). + extend self + + # Used by #set_or_return to avoid emitting a deprecation warning for # "value nil" and to keep default stickiness working exactly the same # @api private diff --git a/lib/chef/property.rb b/lib/chef/property.rb index 8b5c6560a9..437ee10147 100644 --- a/lib/chef/property.rb +++ b/lib/chef/property.rb @@ -87,16 +87,21 @@ class Chef # is fully initialized. # def initialize(**options) - options.each { |k,v| options[k.to_sym] = v if k.is_a?(String) } + options.each { |k,v| options[k.to_sym] = v; options.delete(k) if k.is_a?(String) } + @options = options + options[:name] = options[:name].to_sym if options[:name] + options[:instance_variable_name] = options[:instance_variable_name].to_sym if options[:instance_variable_name] + # Replace name_attribute with name_property if options.has_key?(:name_attribute) # If we have both name_attribute and name_property and they differ, raise an error if options.has_key?(:name_property) - raise ArgumentError, "Cannot specify both name_property and name_attribute together on property #{options[:name]}#{options[:declared_in] ? " of resource #{options[:declared_in].resource_name}" : ""}." + raise ArgumentError, "Cannot specify both name_property and name_attribute together on property #{self}." end # replace name_property with name_attribute in place options = Hash[options.map { |k,v| k == :name_attribute ? [ :name_property, v ] : [ k,v ] }] + @options = options end # Only pick the first of :default, :name_property and :name_attribute if @@ -109,17 +114,22 @@ class Chef options.delete(:name_property) preferred_default = :default end - Chef.log_deprecation("Cannot specify both default and name_property together on property #{options[:name]}#{options[:declared_in] ? " of resource #{options[:declared_in].resource_name}" : ""}. Only one (#{preferred_default}) will be obeyed. In Chef 13, this will become an error.") + Chef.log_deprecation("Cannot specify both default and name_property together on property #{self}. Only one (#{preferred_default}) will be obeyed. In Chef 13, this will become an error. Please remove one or the other from the property.") end - @options = options - - options[:name] = options[:name].to_sym if options[:name] - options[:instance_variable_name] = options[:instance_variable_name].to_sym if options[:instance_variable_name] + # Validate the default early, so the user gets a good error message, and + # cache it so we don't do it again if so + begin + # If we can validate it all the way to output, do it. + @stored_default = input_to_stored_value(nil, default, is_default: true) + rescue Chef::Exceptions::CannotValidateStaticallyError + # If the validation is not static (i.e. has procs), we will have to + # coerce and validate the default each time we run + end end def to_s - name + "#{name}#{declared_in ? " of resource #{declared_in.resource_name}" : ""}" end # @@ -253,14 +263,26 @@ class Chef return get(resource) end - if value.nil? && !explicitly_accepts_nil?(resource) + if value.nil? # In Chef 12, value(nil) does a *get* instead of a set, so we # warn if the value would have been changed. In Chef 13, it will be # equivalent to value = nil. result = get(resource) - if !result.nil? - Chef.log_deprecation("#{name} nil currently does not overwrite the value of #{name}. This will change in Chef 13, and the value will be set to nil instead. Please change your code to explicitly accept nil using \"property :#{name}, [MyType, nil]\", or stop setting this value to nil.") + + # Warn about this becoming a set in Chef 13. + begin + input_to_stored_value(resource, value) + # If nil is valid, and it would change the value, warn that this will change to a set. + if !result.nil? + Chef.log_deprecation("An attempt was made to change #{name} from #{result.inspect} to nil by calling #{name}(nil). In Chef 12, this does a get rather than a set. In Chef 13, this will change to set the value to nil.") + end + rescue Chef::Exceptions::DeprecatedFeatureError + raise + rescue + # If nil is invalid, warn that this will become an error. + Chef.log_deprecation("nil is an invalid value for #{self}. In Chef 13, this warning will change to an error. Error: #{$!}") end + result else # Anything else, such as myprop(value) is a set @@ -291,16 +313,14 @@ class Chef # this property, or if the value is required and not set. # def get(resource) + # If it's set, return it (and evaluate any lazy values) if is_set?(resource) value = get_value(resource) - if value.is_a?(DelayedEvaluator) - value = exec_in_resource(resource, value) - value = coerce(resource, value) - validate(resource, value) - end - value + value = stored_value_to_output(resource, value) else + # We are getting the default value. + # If the user does something like this: # # ``` @@ -326,14 +346,14 @@ class Chef end if has_default? - value = default - if value.is_a?(DelayedEvaluator) - value = exec_in_resource(resource, value) + # If we were able to cache the stored_default, grab it. + if defined?(@stored_default) + value = @stored_default + else + # Otherwise, we have to validate it now. + value = input_to_stored_value(resource, default, is_default: true) end - - value = coerce(resource, value) - - # We don't validate defaults + value = stored_value_to_output(resource, value, is_default: true) # If the value is mutable (non-frozen), we set it on the instance # so that people can mutate it. (All constant default values are @@ -366,11 +386,7 @@ class Chef # this property. # def set(resource, value) - unless value.is_a?(DelayedEvaluator) - value = coerce(resource, value) - validate(resource, value) - end - set_value(resource, value) + set_value(resource, input_to_stored_value(resource, value)) end # @@ -423,7 +439,10 @@ class Chef # def coerce(resource, value) if options.has_key?(:coerce) - value = exec_in_resource(resource, options[:coerce], value) + # If we have no default value, `nil` is never coerced or validated + unless !has_default? && value.nil? + value = exec_in_resource(resource, options[:coerce], value) + end end value end @@ -442,7 +461,10 @@ class Chef # this property. # def validate(resource, value) - resource.validate({ name => value }, { name => validation_options }) + # If we have no default value, `nil` is never coerced or validated + unless value.nil? && !has_default? + (resource || Chef::Mixin::ParamsValidate).validate({ name => value }, { name => validation_options }) + end end # @@ -595,14 +617,53 @@ class Chef value = resource.instance_exec(*args, &proc) end else - value = proc.call + # If we don't have a resource yet, we can't exec in resource! + raise Chef::Exceptions::CannotValidateStaticallyError, "Cannot validate or coerce without a resource" + end + end + + def input_to_stored_value(resource, value, is_default: false) + unless value.is_a?(DelayedEvaluator) + value = coerce_and_validate(resource, value, is_default: is_default) end + value + end + def stored_value_to_output(resource, value, is_default: false) + # Crack open lazy values before giving the result to the user if value.is_a?(DelayedEvaluator) - value = coerce(resource, value) - validate(resource, value) + value = exec_in_resource(resource, value) + value = coerce_and_validate(resource, value, is_default: is_default) end value end + + # Coerces and validates the value. If the value is a default, it will warn + # the user that invalid defaults are bad mmkay, and return it as if it were + # valid. + def coerce_and_validate(resource, value, is_default: false) + result = coerce(resource, value) + begin + # If the input is from a default, we need to emit an invalid default warning on validate. + validate(resource, result) + rescue Chef::Exceptions::CannotValidateStaticallyError + # This one gets re-raised + raise + rescue + # Anything else is just an invalid default: in those cases, we just + # warn and return the (possibly coerced) value to the user. + if is_default + if value.nil? + Chef.log_deprecation("Default value nil is invalid for property #{self}. Possible fixes: 1. Remove 'default: nil' if nil means 'undefined'. 2. Set a valid default value if there is a reasonable one. 3. Allow nil as a valid value of your property (for example, 'property #{name.inspect}, [ String, nil ], default: nil'). Error: #{$!}") + else + Chef.log_deprecation("Default value #{value.inspect} is invalid for property #{self}. In Chef 13 this will become an error: #{$!}.") + end + else + raise + end + end + + result + end end end diff --git a/lib/chef/resource/chef_gem.rb b/lib/chef/resource/chef_gem.rb index 233ae66026..655534684c 100644 --- a/lib/chef/resource/chef_gem.rb +++ b/lib/chef/resource/chef_gem.rb @@ -26,9 +26,9 @@ class Chef property :gem_binary, default: "#{RbConfig::CONFIG['bindir']}/gem", callbacks: { - "The chef_gem resource is restricted to the current gem environment, use gem_package to install to other environments." => proc { false } + "The chef_gem resource is restricted to the current gem environment, use gem_package to install to other environments." => proc { |v| v == properties[:gem_binary].default } } - property :compile_time, [ true, false ], default: lazy { Chef::Config[:chef_gem_compile_time] }, desired_state: false + property :compile_time, [ true, false, nil ], default: lazy { Chef::Config[:chef_gem_compile_time] }, desired_state: false def after_created # Chef::Resource.run_action: Caveat: this skips Chef::Runner.run_action, where notifications are handled diff --git a/lib/chef/resource/file.rb b/lib/chef/resource/file.rb index 0a530e27c9..52cb409226 100644 --- a/lib/chef/resource/file.rb +++ b/lib/chef/resource/file.rb @@ -49,7 +49,7 @@ class Chef allowed_actions :create, :delete, :touch, :create_if_missing property :path, String, name_property: true, identity: true - property :atomic_update, [ true, false ], desired_state: false, default: Chef::Config[:file_atomic_update] + property :atomic_update, [ true, false ], desired_state: false, default: lazy { Chef::Config[:file_atomic_update] } property :backup, [ Integer, false ], desired_state: false, default: 5 property :checksum, [ /^[a-zA-Z0-9]{64}$/, nil ] property :content, [ String, nil ], desired_state: false |