summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLamont Granquist <lamont@scriptkiddie.org>2020-04-17 12:51:13 -0700
committerLamont Granquist <lamont@scriptkiddie.org>2020-04-17 16:23:51 -0700
commit49aa17ad9805b6949baaa37cfe66f8cf2b8083a8 (patch)
tree0c1d22d58c8c679c048aac2b400a1b5703c0a70a
parent8feb3f054185d01d70e42e917429c692dfd8dc81 (diff)
downloadchef-49aa17ad9805b6949baaa37cfe66f8cf2b8083a8.tar.gz
Force requiring properties
All required properties are now required for all actions by default even if the action does not reference the property. In order to only make the property required for a subset of the actions, specify them as an array of symbols to the required options on the property. ``` property :whatever, String, required: %i{start stop} action :start do end action :stop do end action :enable do end action :disable do end ``` That property will be required for start+stop but not for enable+disable. There's an unaddressed edge case here where if you reference the property in an action which was not specified that it will also fail validation. That is correct behavior. We should probably dig into how to warn the user that they must either remove the reference to the property from that action or else to add the action to the list of required actions on the property. Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>
-rw-r--r--lib/chef/property.rb8
-rw-r--r--lib/chef/provider.rb11
-rw-r--r--spec/unit/property/validation_spec.rb26
3 files changed, 41 insertions, 4 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/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