diff options
author | Noah Kantrowitz <noah@coderanger.net> | 2016-08-22 18:54:44 -0700 |
---|---|---|
committer | Noah Kantrowitz <noah@coderanger.net> | 2016-08-22 18:54:44 -0700 |
commit | bea83309ccedfe48be5ebac979bb531100f31883 (patch) | |
tree | 5f84b406057efec51953ad6393b4b293e5a82de9 | |
parent | 1429df950b0a6865d55f6e432a3aa0c615e1f1a7 (diff) | |
download | chef-bea83309ccedfe48be5ebac979bb531100f31883.tar.gz |
Add a warning for guard blocks that return a non-empty string.
This will hopefully catch errors like this:
myresource 'name' do
not_if { 'some command' }
end
-rw-r--r-- | lib/chef/resource/conditional.rb | 10 | ||||
-rw-r--r-- | spec/unit/resource/conditional_spec.rb | 46 |
2 files changed, 55 insertions, 1 deletions
diff --git a/lib/chef/resource/conditional.rb b/lib/chef/resource/conditional.rb index cdb9f13c45..452718cae8 100644 --- a/lib/chef/resource/conditional.rb +++ b/lib/chef/resource/conditional.rb @@ -103,7 +103,15 @@ class Chef end def evaluate_block - @block.call + @block.call.tap do |rv| + if rv.is_a?(String) && !rv.empty? + # This is probably a mistake: + # not_if { "command" } + sanitized_rv = @parent_resource.sensitive ? "a string" : rv.inspect + Chef::Log.warn("#{@positivity} block for #{@parent_resource} returned #{sanitized_rv}, did you mean to run a command?" + + (@parent_resource.sensitive ? "" : " If so use '#{@positivity} #{sanitized_rv}' in your code.")) + end + end end def short_description diff --git a/spec/unit/resource/conditional_spec.rb b/spec/unit/resource/conditional_spec.rb index b34b4200e6..e84b0980c4 100644 --- a/spec/unit/resource/conditional_spec.rb +++ b/spec/unit/resource/conditional_spec.rb @@ -124,6 +124,29 @@ describe Chef::Resource::Conditional do expect(@conditional.continue?).to be_falsey end end + + describe "after running a block that returns a string value" do + before do + @conditional = Chef::Resource::Conditional.only_if(@parent_resource) { "some command" } + end + + it "logs a warning" do + expect(Chef::Log).to receive(:warn).with("only_if block for [] returned \"some command\", did you mean to run a command? If so use 'only_if \"some command\"' in your code.") + @conditional.evaluate + end + end + + describe "after running a block that returns a string value on a sensitive resource" do + before do + @parent_resource.sensitive(true) + @conditional = Chef::Resource::Conditional.only_if(@parent_resource) { "some command" } + end + + it "logs a warning" do + expect(Chef::Log).to receive(:warn).with("only_if block for [] returned a string, did you mean to run a command?") + @conditional.evaluate + end + end end describe "when created as a `not_if`" do @@ -204,5 +227,28 @@ describe Chef::Resource::Conditional do expect(@conditional.continue?).to be_truthy end end + + describe "after running a block that returns a string value" do + before do + @conditional = Chef::Resource::Conditional.not_if(@parent_resource) { "some command" } + end + + it "logs a warning" do + expect(Chef::Log).to receive(:warn).with("not_if block for [] returned \"some command\", did you mean to run a command? If so use 'not_if \"some command\"' in your code.") + @conditional.evaluate + end + end + + describe "after running a block that returns a string value on a sensitive resource" do + before do + @parent_resource.sensitive(true) + @conditional = Chef::Resource::Conditional.not_if(@parent_resource) { "some command" } + end + + it "logs a warning" do + expect(Chef::Log).to receive(:warn).with("not_if block for [] returned a string, did you mean to run a command?") + @conditional.evaluate + end + end end end |