diff options
author | Tim Smith <tsmith@chef.io> | 2020-04-17 17:53:40 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-04-17 17:53:40 -0700 |
commit | 272bafca5a132aa79c804a160f04a091529b0396 (patch) | |
tree | 061b5e648c74c87ac1060b43896be9ff191823d8 | |
parent | 8feb3f054185d01d70e42e917429c692dfd8dc81 (diff) | |
parent | 4fc5f4cc93e252bea005a1822b5a9220734e659c (diff) | |
download | chef-272bafca5a132aa79c804a160f04a091529b0396.tar.gz |
Merge pull request #9688 from chef/lcg/required-properties-chef-16
Force requiring properties
-rw-r--r-- | lib/chef/property.rb | 8 | ||||
-rw-r--r-- | lib/chef/provider.rb | 11 | ||||
-rw-r--r-- | lib/chef/resource/windows_env.rb | 2 | ||||
-rw-r--r-- | spec/unit/property/validation_spec.rb | 26 |
4 files changed, 42 insertions, 5 deletions
diff --git a/lib/chef/property.rb b/lib/chef/property.rb index eaf1a66e86..d65545c138 100644 --- a/lib/chef/property.rb +++ b/lib/chef/property.rb @@ -303,8 +303,12 @@ class Chef # # @return [Boolean] # - def required? - options[:required] + def required?(action = nil) + if !action.nil? && options[:required].is_a?(Array) + options[:required].include?(action) + else + !!options[:required] + end end # diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb index 40bd5a48a1..947a890806 100644 --- a/lib/chef/provider.rb +++ b/lib/chef/provider.rb @@ -152,6 +152,7 @@ class Chef new_resource.cookbook_name end + # hook that subclasses can use to do lazy validation for where properties aren't flexibile enough def check_resource_semantics!; end # a simple placeholder method that will be called / raise if a resource tries to @@ -181,12 +182,20 @@ class Chef run_context.events end + def validate_required_properties! + # all we do is run through all the required properties for this action and vivify them + new_resource.class.properties.each { |name, property| property.required?(action) && property.get(new_resource) } + end + def run_action(action = nil) @action = action unless action.nil? - # @todo it would be preferable to get the action to be executed in the constructor... + # hook that subclasses can use to do lazy validation for where properties aren't flexibile enough check_resource_semantics! + # force the validation of required properties + validate_required_properties! + # user-defined LWRPs may include unsafe load_current_resource methods that cannot be run in whyrun mode if whyrun_mode? && !whyrun_supported? events.resource_current_state_load_bypassed(@new_resource, @action, @current_resource) diff --git a/lib/chef/resource/windows_env.rb b/lib/chef/resource/windows_env.rb index b7f347b80b..8f551df9f9 100644 --- a/lib/chef/resource/windows_env.rb +++ b/lib/chef/resource/windows_env.rb @@ -36,7 +36,7 @@ class Chef property :value, String, description: "The value of the environmental variable to set.", - required: true + required: %i{create modify} property :delim, [ String, nil, false ], description: "The delimiter that is used to separate multiple values for a single key.", diff --git a/spec/unit/property/validation_spec.rb b/spec/unit/property/validation_spec.rb index dab2be8000..c8daee1580 100644 --- a/spec/unit/property/validation_spec.rb +++ b/spec/unit/property/validation_spec.rb @@ -45,11 +45,22 @@ describe "Chef::Resource.property validation" do def self.blah "class#{Namer.next_index}" end + + action :doit do + # this needs to not reference any properties + end + + action :doit2 do + # this needs to not reference any properties + end end end + let(:node) { Chef::Node.new } + let(:events) { Chef::EventDispatch::Dispatcher.new } + let(:run_context) { Chef::RunContext.new(node, {}, events) } let(:resource) do - resource_class.new("blah") + resource_class.new("blah", run_context) end def self.english_join(values) @@ -576,6 +587,19 @@ describe "Chef::Resource.property validation" do it "value nil emits a validation failed error because it must have a value" do expect { resource.x nil }.to raise_error Chef::Exceptions::ValidationFailed end + it "fails if it is not specified, on running the action, even if it is not referenced" do + expect { resource.run_action(:doit) }.to raise_error Chef::Exceptions::ValidationFailed + end + end + + with_property ":x, required: %i{doit}" do + it "fails if it is not specified, on running the doit action, even if it is not referenced" do + expect { resource.run_action(:doit) }.to raise_error Chef::Exceptions::ValidationFailed + end + + it "does not fail if it is not specified, on running the doit2 action" do + expect { resource.run_action(:doit2) }.not_to raise_error + end end with_property ":x, String, required: true" do |