Merge pull request #12282 from chef/lcg/define-resource-requirements-fix

16 files changed, 87 insertions, 13 deletions

diff --git a/lib/chef/mixin/why_run.rb b/lib/chef/mixin/why_run.rb
index efe327168e..357c4a655d 100644
--- a/lib/chef/mixin/why_run.rb
+++ b/lib/chef/mixin/why_run.rb
@@ -242,8 +242,12 @@ class Chef
           end
         end
 
-        def initialize(resource, run_context)
-          @resource, @run_context = resource, run_context
+        attr_accessor :action
+
+        def initialize(resource, run_context, action)
+          @resource = resource
+          @run_context = run_context
+          @action = action
           @assertions = Hash.new { |h, k| h[k] = [] }
           @blocked_actions = []
         end
@@ -305,6 +309,8 @@ class Chef
         #                       "You don't have sufficient privileges to delete #{@new_resource.path}")
         #   end
         def assert(*actions)
+          return unless actions.include?(action.to_sym) || actions.include?(:all_actions)
+
           assertion = Assertion.new
           yield assertion
           actions.each { |action| @assertions[action] << assertion }
diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index e7d7ca84ff..80c58c09ee 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -269,7 +269,7 @@ class Chef
     end
 
     def requirements
-      @requirements ||= ResourceRequirements.new(@new_resource, run_context)
+      @requirements ||= ResourceRequirements.new(@new_resource, run_context, action || new_resource.action)
     end
 
     def description(description = "NOT_PASSED")
diff --git a/spec/unit/file_access_control_spec.rb b/spec/unit/file_access_control_spec.rb
index dfa0bcf673..3b533ec014 100644
--- a/spec/unit/file_access_control_spec.rb
+++ b/spec/unit/file_access_control_spec.rb
@@ -35,7 +35,7 @@ describe Chef::FileAccessControl do
         @events = Chef::EventDispatch::Dispatcher.new
         @run_context = Chef::RunContext.new(@node, {}, @events)
         @current_resource = Chef::Resource::File.new("/tmp/different_file.txt")
-        @provider_requirements = Chef::Provider::ResourceRequirements.new(@resource, @run_context)
+        @provider_requirements = Chef::Provider::ResourceRequirements.new(@resource, @run_context, :create)
         @provider = double("File provider", requirements: @provider_requirements, manage_symlink_access?: false)
 
         @fac = Chef::FileAccessControl.new(@current_resource, @resource, @provider)
diff --git a/spec/unit/mixin/why_run_spec.rb b/spec/unit/mixin/why_run_spec.rb
new file mode 100644
index 0000000000..7e56433fea
--- /dev/null
+++ b/spec/unit/mixin/why_run_spec.rb
@@ -0,0 +1,53 @@
+#
+# Copyright:: Copyright (c) Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+require "spec_helper"
+
+describe Chef::Mixin::WhyRun::ResourceRequirements do
+  class TestResource < Chef::Resource
+    action_class do
+      def define_resource_requirements
+        requirements.assert(:boom) do |a|
+          a.assertion { raise "boom1" }
+          a.failure_message("#{raise "boom2"}")
+          a.whyrun("#{raise "boom3"}")
+        end
+      end
+    end
+
+    action :boom do
+      # nothing
+    end
+
+    action :noboom do
+      # nothing
+    end
+  end
+
+  let(:node) { Chef::Node.new }
+  let(:events) { Chef::EventDispatch::Dispatcher.new }
+  let(:run_context) { Chef::RunContext.new(node, {}, events) }
+  let(:resource) { TestResource.new("name", run_context) }
+
+  it "raises an exception for an action where the assertions raise exceptions" do
+    expect { resource.run_action(:boom) }.to raise_error(StandardError, /boom2/)
+  end
+
+  it "does not raise an exception for an action which has no assertions" do
+    resource.run_action(:noboom)
+  end
+end
diff --git a/spec/unit/provider/group/groupadd_spec.rb b/spec/unit/provider/group/groupadd_spec.rb
index 50ee766cdb..70e5b4a39e 100644
--- a/spec/unit/provider/group/groupadd_spec.rb
+++ b/spec/unit/provider/group/groupadd_spec.rb
@@ -169,6 +169,7 @@ describe Chef::Provider::Group::Groupadd do
 
     before do
       allow(File).to receive(:exist?).and_return(false)
+      provider.action = :modify
       provider.define_resource_requirements
     end
 
diff --git a/spec/unit/provider/group/usermod_spec.rb b/spec/unit/provider/group/usermod_spec.rb
index e552516063..caac1857cf 100644
--- a/spec/unit/provider/group/usermod_spec.rb
+++ b/spec/unit/provider/group/usermod_spec.rb
@@ -58,18 +58,18 @@ describe Chef::Provider::Group::Usermod do
       end
 
       it "should raise an error when setting the entire group directly" do
+        @provider.action = :modify
         @provider.define_resource_requirements
         @provider.load_current_resource
         @provider.instance_variable_set("@group_exists", true)
-        @provider.action = :modify
         expect { @provider.run_action(@provider.process_resource_requirements) }.to raise_error(Chef::Exceptions::Group, "setting group members directly is not supported by #{@provider}, must set append true in group")
       end
 
       it "should raise an error when excluded_members are set" do
+        @provider.action = :modify
         @provider.define_resource_requirements
         @provider.load_current_resource
         @provider.instance_variable_set("@group_exists", true)
-        @provider.action = :modify
         @new_resource.append(true)
         @new_resource.excluded_members(["someone"])
         expect { @provider.run_action(@provider.process_resource_requirements) }.to raise_error(Chef::Exceptions::Group, "excluded_members is not supported by #{@provider}")
diff --git a/spec/unit/provider/ifconfig_spec.rb b/spec/unit/provider/ifconfig_spec.rb
index 668a3ca9d9..166fe24304 100644
--- a/spec/unit/provider/ifconfig_spec.rb
+++ b/spec/unit/provider/ifconfig_spec.rb
@@ -61,6 +61,7 @@ describe Chef::Provider::Ifconfig do
         expect(@provider.instance_variable_get("@status").exitstatus).not_to eq(0)
       end
       it "should thrown an exception when ifconfig fails" do
+        @provider.action = :add
         @provider.define_resource_requirements
         expect { @provider.process_resource_requirements }.to raise_error Chef::Exceptions::Ifconfig
       end
@@ -81,6 +82,7 @@ describe Chef::Provider::Ifconfig do
         expect(@provider.instance_variable_get("@status").exitstatus).not_to eq(0)
       end
       it "should thrown an exception when ifconfig fails" do
+        @provider.action = :add
         @provider.define_resource_requirements
         expect { @provider.process_resource_requirements }.to raise_error Chef::Exceptions::Ifconfig
       end
diff --git a/spec/unit/provider/package/bff_spec.rb b/spec/unit/provider/package/bff_spec.rb
index 680e5cf22a..b32b7714e4 100644
--- a/spec/unit/provider/package/bff_spec.rb
+++ b/spec/unit/provider/package/bff_spec.rb
@@ -61,6 +61,7 @@ describe Chef::Provider::Package::Bff do
       allow(@provider).to receive(:shell_out_compacted).and_return(@empty_status)
       allow(::File).to receive(:exist?).with(@new_resource.source).and_return(false)
       @provider.load_current_resource
+      @provider.action = :install
       @provider.define_resource_requirements
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Package)
     end
diff --git a/spec/unit/provider/package/solaris_spec.rb b/spec/unit/provider/package/solaris_spec.rb
index c07367c221..874f030605 100644
--- a/spec/unit/provider/package/solaris_spec.rb
+++ b/spec/unit/provider/package/solaris_spec.rb
@@ -65,6 +65,7 @@ describe Chef::Provider::Package::Solaris do
       allow(@provider).to receive(:shell_out_compacted).and_return(@status)
       allow(::File).to receive(:exist?).and_return(false)
       @provider.load_current_resource
+      @provider.action = :install
       @provider.define_resource_requirements
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Package)
     end
diff --git a/spec/unit/provider/service/arch_service_spec.rb b/spec/unit/provider/service/arch_service_spec.rb
index c92af83de5..496e876a49 100644
--- a/spec/unit/provider/service/arch_service_spec.rb
+++ b/spec/unit/provider/service/arch_service_spec.rb
@@ -95,15 +95,15 @@ describe Chef::Provider::Service::Arch, "load_current_resource" do
 
   it "should raise error if the node has a nil ps property and no other means to get status" do
     @node.automatic_attrs[:command] = { ps: nil }
-    @provider.define_resource_requirements
     @provider.action = :start
+    @provider.define_resource_requirements
     expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
   end
 
   it "should raise error if the node has an empty ps property and no other means to get status" do
     @node.automatic_attrs[:command] = { ps: "" }
-    @provider.define_resource_requirements
     @provider.action = :start
+    @provider.define_resource_requirements
     expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
   end
 
diff --git a/spec/unit/provider/service/debian_service_spec.rb b/spec/unit/provider/service/debian_service_spec.rb
index 0ae1d28cd3..c3a478d249 100644
--- a/spec/unit/provider/service/debian_service_spec.rb
+++ b/spec/unit/provider/service/debian_service_spec.rb
@@ -50,6 +50,7 @@ describe Chef::Provider::Service::Debian do
     it "ensures /usr/sbin/update-rc.d is available" do
       expect(File).to receive(:exist?).with("/usr/sbin/update-rc.d").and_return(false)
 
+      @provider.action = :start
       @provider.define_resource_requirements
       expect do
         @provider.process_resource_requirements
diff --git a/spec/unit/provider/service/gentoo_service_spec.rb b/spec/unit/provider/service/gentoo_service_spec.rb
index f195fbe40b..efab722ac8 100644
--- a/spec/unit/provider/service/gentoo_service_spec.rb
+++ b/spec/unit/provider/service/gentoo_service_spec.rb
@@ -42,6 +42,7 @@ describe Chef::Provider::Service::Gentoo do
   describe "load_current_resource" do
     it "should raise Chef::Exceptions::Service if /sbin/rc-update does not exist" do
       expect(File).to receive(:exist?).with("/sbin/rc-update").and_return(false)
+      @provider.action = :start
       @provider.define_resource_requirements
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
     end
diff --git a/spec/unit/provider/service/macosx_spec.rb b/spec/unit/provider/service/macosx_spec.rb
index eafc857cf1..ebde71776a 100644
--- a/spec/unit/provider/service/macosx_spec.rb
+++ b/spec/unit/provider/service/macosx_spec.rb
@@ -237,6 +237,7 @@ describe Chef::Provider::Service::Macosx do
                   allow(Dir).to receive(:glob).and_return([(plist).to_s,
                                               "/Users/wtf/something.plist"])
                   provider.load_current_resource
+                  provider.action = :enable
                   provider.define_resource_requirements
                   expect { provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
                 end
diff --git a/spec/unit/provider/service/redhat_spec.rb b/spec/unit/provider/service/redhat_spec.rb
index d5d2c7ddc0..1bf07b7f2c 100644
--- a/spec/unit/provider/service/redhat_spec.rb
+++ b/spec/unit/provider/service/redhat_spec.rb
@@ -34,6 +34,7 @@ shared_examples_for "define_resource_requirements_common" do
     expect(@provider).to receive(:shell_out).with("/sbin/service chef status").and_return(status)
     chkconfig = double("Chkconfig", exitstatus: 0, stdout: "", stderr: "service chef supports chkconfig, but is not referenced in any runlevel (run 'chkconfig --add chef')")
     expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", returns: [0, 1]).and_return(chkconfig)
+    @provider.action = :start
     @provider.load_current_resource
     @provider.define_resource_requirements
     expect { @provider.process_resource_requirements }.not_to raise_error
@@ -147,12 +148,12 @@ describe "Chef::Provider::Service::Redhat" do
           chkconfig = double("Chkconfig", existatus: 1, stdout: "", stderr: "error reading information on service chef: No such file or directory")
           expect(@provider).to receive(:shell_out!).with("/sbin/chkconfig --list chef", returns: [0, 1]).and_return(chkconfig)
           @provider.load_current_resource
-          @provider.define_resource_requirements
         end
 
         %w{start reload restart enable}.each do |action|
           it "should raise an error when the action is #{action}" do
             @provider.action = action
+            @provider.define_resource_requirements
             expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
           end
         end
@@ -161,12 +162,14 @@ describe "Chef::Provider::Service::Redhat" do
           it "should not raise an error when the action is #{action} and init_command is set" do
             @new_resource.init_command("/etc/init.d/chef")
             @provider.action = action
+            @provider.define_resource_requirements
             expect { @provider.process_resource_requirements }.not_to raise_error
           end
 
           it "should not raise an error when the action is #{action} and #{action}_command is set" do
             @new_resource.send("#{action}_command", "/etc/init.d/chef #{action}")
             @provider.action = action
+            @provider.define_resource_requirements
             expect { @provider.process_resource_requirements }.not_to raise_error
           end
         end
diff --git a/spec/unit/provider/service/simple_service_spec.rb b/spec/unit/provider/service/simple_service_spec.rb
index 2546f9cef7..e114d3e314 100644
--- a/spec/unit/provider/service/simple_service_spec.rb
+++ b/spec/unit/provider/service/simple_service_spec.rb
@@ -52,12 +52,14 @@ describe Chef::Provider::Service::Simple, "load_current_resource" do
 
   it "should raise error if the node has a nil ps property and no other means to get status" do
     @node.automatic_attrs[:command] = { ps: nil }
+    @provider.action = :start
     @provider.define_resource_requirements
     expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
   end
 
   it "should raise error if the node has an empty ps property and no other means to get status" do
     @node.automatic_attrs[:command] = { ps: "" }
+    @provider.action = :start
     @provider.define_resource_requirements
     expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
   end
@@ -112,8 +114,8 @@ describe Chef::Provider::Service::Simple, "load_current_resource" do
     end
 
     it "should raise an exception if no start command is specified" do
-      @provider.define_resource_requirements
       @provider.action = :start
+      @provider.define_resource_requirements
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
     end
   end
@@ -126,8 +128,8 @@ describe Chef::Provider::Service::Simple, "load_current_resource" do
     end
 
     it "should raise an exception if no stop command is specified" do
-      @provider.define_resource_requirements
       @provider.action = :stop
+      @provider.define_resource_requirements
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
     end
   end
@@ -140,8 +142,8 @@ describe Chef::Provider::Service::Simple, "load_current_resource" do
     end
 
     it "should raise an exception if the resource doesn't support restart, no restart command is provided, and no stop command is provided" do
-      @provider.define_resource_requirements
       @provider.action = :restart
+      @provider.define_resource_requirements
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::Service)
     end
 
@@ -155,8 +157,8 @@ describe Chef::Provider::Service::Simple, "load_current_resource" do
 
   describe Chef::Provider::Service::Simple, "reload_service" do
     it "should raise an exception if reload is requested but no command is specified" do
-      @provider.define_resource_requirements
       @provider.action = :reload
+      @provider.define_resource_requirements
       expect { @provider.process_resource_requirements }.to raise_error(Chef::Exceptions::UnsupportedAction)
     end
 
diff --git a/spec/unit/provider/user_spec.rb b/spec/unit/provider/user_spec.rb
index 16428de1a9..c8ad656f06 100644
--- a/spec/unit/provider/user_spec.rb
+++ b/spec/unit/provider/user_spec.rb
@@ -170,6 +170,7 @@ describe Chef::Provider::User do
                                                     sp_warn: 7, sp_inact: -1, sp_expire: -1, sp_flag: -1)
               expect(Shadow::Passwd).to receive(:getspnam).with("notarealuser").and_return(passwd_info)
               @provider.load_current_resource
+              @provider.action = :create
               @provider.define_resource_requirements
               @provider.process_resource_requirements
             end
@@ -180,6 +181,7 @@ describe Chef::Provider::User do
       it "should fail assertions when ruby-shadow cannot be loaded" do
         expect(@provider).to receive(:require).with("shadow") { raise LoadError }
         @provider.load_current_resource
+        @provider.action = :create
         @provider.define_resource_requirements
         expect { @provider.process_resource_requirements }.to raise_error Chef::Exceptions::MissingLibrary
       end