Merge pull request #3360 from chef/jdm/resource-semantics

Add check_resource_semantics! lifecycle method to provider

4 files changed, 37 insertions, 0 deletions

diff --git a/lib/chef/provider.rb b/lib/chef/provider.rb
index 3ab4d4d2b1..99d09d0507 100644
--- a/lib/chef/provider.rb
+++ b/lib/chef/provider.rb
@@ -83,6 +83,9 @@ class Chef
       new_resource.cookbook_name
     end
 
+    def check_resource_semantics!
+    end
+
     def load_current_resource
       raise Chef::Exceptions::Override, "You must override load_current_resource in #{self.to_s}"
     end
@@ -108,6 +111,8 @@ class Chef
       # TODO: it would be preferable to get the action to be executed in the
       # constructor...
 
+      check_resource_semantics!
+
       # user-defined LWRPs may include unsafe load_current_resource methods that cannot be run in whyrun mode
       if !whyrun_mode? || whyrun_supported?
         load_current_resource
diff --git a/lib/chef/provider/package.rb b/lib/chef/provider/package.rb
index 6b429a400d..bdc96cd070 100644
--- a/lib/chef/provider/package.rb
+++ b/lib/chef/provider/package.rb
@@ -43,6 +43,12 @@ class Chef
         true
       end
 
+      def check_resource_semantics!
+        if new_resource.package_name.is_a?(Array) && new_resource.source != nil
+          raise Chef::Exceptions::InvalidResourceSpecification, "You may not specify both multipackage and source"
+        end
+      end
+
       def load_current_resource
       end
 
diff --git a/spec/unit/provider/package_spec.rb b/spec/unit/provider/package_spec.rb
index 1633d18f9d..c6ec0fb3cb 100644
--- a/spec/unit/provider/package_spec.rb
+++ b/spec/unit/provider/package_spec.rb
@@ -37,6 +37,12 @@ describe Chef::Provider::Package do
       allow(@provider).to receive(:install_package).and_return(true)
     end
 
+    it "raises a Chef::Exceptions::InvalidResourceSpecification if both multipackage and source are provided" do
+      @new_resource.package_name(['a', 'b'])
+      @new_resource.source('foo')
+      expect { @provider.run_action(:install) }.to raise_error(Chef::Exceptions::InvalidResourceSpecification)
+    end
+
     it "should raise a Chef::Exceptions::Package if no version is specified, and no candidate is available" do
       @provider.candidate_version = nil
       expect { @provider.run_action(:install) }.to raise_error(Chef::Exceptions::Package)
diff --git a/spec/unit/provider_spec.rb b/spec/unit/provider_spec.rb
index 5a21b094d0..d7a34bc21b 100644
--- a/spec/unit/provider_spec.rb
+++ b/spec/unit/provider_spec.rb
@@ -49,6 +49,13 @@ class ConvergeActionDemonstrator < Chef::Provider
   end
 end
 
+class CheckResourceSemanticsDemonstrator < ConvergeActionDemonstrator
+  def check_resource_semantics!
+    raise Chef::Exceptions::InvalidResourceSpecification.new("check_resource_semantics!")
+  end
+end
+
+
 describe Chef::Provider do
   before(:each) do
     @cookbook_collection = Chef::CookbookCollection.new([])
@@ -89,6 +96,10 @@ describe Chef::Provider do
     expect(@provider.send(:whyrun_supported?)).to eql(false)
   end
 
+  it "should do nothing for check_resource_semantics! by default" do
+    expect { @provider.check_resource_semantics! }.not_to raise_error
+  end
+
   it "should return true for action_nothing" do
     expect(@provider.action_nothing).to eql(true)
   end
@@ -176,6 +187,15 @@ describe Chef::Provider do
         expect(@resource).not_to be_updated_by_last_action
       end
     end
+
+    describe "and the resource is invalid" do
+      let(:provider) { CheckResourceSemanticsDemonstrator.new(@resource, @run_context) }
+
+      it "fails with InvalidResourceSpecification when run" do
+        expect { provider.run_action(:foo) }.to raise_error(Chef::Exceptions::InvalidResourceSpecification)
+      end
+
+    end
   end
 
 end