Merge pull request #3541 from chef/jk/3535

Ensure :nothing is in the list of allowed actions for an LWRP

4 files changed, 92 insertions, 8 deletions

diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb
index 7fe8a52d95..e89dede609 100644
--- a/lib/chef/resource.rb
+++ b/lib/chef/resource.rb
@@ -965,7 +965,7 @@ class Chef
     #
     # @param actions [Array<Symbol>] The list of actions to add to allowed_actions.
     #
-    # @return [Arrau<Symbol>] The list of actions, as symbols.
+    # @return [Array<Symbol>] The list of actions, as symbols.
     #
     def self.allowed_actions(*actions)
       @allowed_actions ||=
@@ -974,10 +974,10 @@ class Chef
         else
           [ :nothing ]
         end
-      @allowed_actions |= actions
+      @allowed_actions |= actions.flatten
     end
     def self.allowed_actions=(value)
-      @allowed_actions = value
+      @allowed_actions = value.uniq
     end
 
     #
diff --git a/lib/chef/resource/lwrp_base.rb b/lib/chef/resource/lwrp_base.rb
index c486233020..ef3c2b5bba 100644
--- a/lib/chef/resource/lwrp_base.rb
+++ b/lib/chef/resource/lwrp_base.rb
@@ -85,8 +85,9 @@ class Chef
         # Adds +action_names+ to the list of valid actions for this resource.
         # Does not include superclass's action list when appending.
         def actions(*action_names)
+          action_names = action_names.flatten
           if !action_names.empty? && !@allowed_actions
-            self.allowed_actions = action_names
+            self.allowed_actions = ([ :nothing ] + action_names).uniq
           else
             allowed_actions(*action_names)
           end
diff --git a/spec/integration/recipes/recipe_dsl_spec.rb b/spec/integration/recipes/recipe_dsl_spec.rb
index 3f4bf9fd5f..05ae72e688 100644
--- a/spec/integration/recipes/recipe_dsl_spec.rb
+++ b/spec/integration/recipes/recipe_dsl_spec.rb
@@ -754,4 +754,48 @@ describe "Recipe DSL methods" do
       end
     end
   end
+
+  before(:all) { Namer.current_index = 0 }
+  before { Namer.current_index += 1 }
+
+  context "with an LWRP that declares actions" do
+    let(:resource_class) {
+      Class.new(Chef::Resource::LWRPBase) do
+        provides :"recipe_dsl_spec#{Namer.current_index}"
+        actions :create
+      end
+    }
+    let(:resource) {
+      resource_class.new("blah", run_context)
+    }
+    it "The actions are part of actions along with :nothing" do
+      expect(resource_class.actions).to eq [ :nothing, :create ]
+    end
+    it "The actions are part of allowed_actions along with :nothing" do
+      expect(resource.allowed_actions).to eq [ :nothing, :create ]
+    end
+
+    context "and a subclass that declares more actions" do
+      let(:subresource_class) {
+        Class.new(Chef::Resource::LWRPBase) do
+          provides :"recipe_dsl_spec_sub#{Namer.current_index}"
+          actions :delete
+        end
+      }
+      let(:subresource) {
+        subresource_class.new("subblah", run_context)
+      }
+
+      it "The parent class actions are not part of actions" do
+        expect(subresource_class.actions).to eq [ :nothing, :delete ]
+      end
+      it "The parent class actions are not part of allowed_actions" do
+        expect(subresource.allowed_actions).to eq [ :nothing, :delete ]
+      end
+      it "The parent class actions do not change" do
+        expect(resource_class.actions).to eq [ :nothing, :create ]
+        expect(resource.allowed_actions).to eq [ :nothing, :create ]
+      end
+    end
+  end
 end
diff --git a/spec/unit/lwrp_spec.rb b/spec/unit/lwrp_spec.rb
index 34c6f6f1c5..784ff966cd 100644
--- a/spec/unit/lwrp_spec.rb
+++ b/spec/unit/lwrp_spec.rb
@@ -409,13 +409,13 @@ describe "LWRP" do
         end
       end
 
-      context "when the child does not defined the methods" do
+      context "when the child does not define the methods" do
         let(:child) do
           Class.new(parent)
         end
 
         it "delegates #actions to the parent" do
-          expect(child.actions).to eq([:eat, :sleep])
+          expect(child.actions).to eq([:nothing, :eat, :sleep])
         end
 
         it "delegates #default_action to the parent" do
@@ -432,7 +432,7 @@ describe "LWRP" do
         end
 
         it "does not delegate #actions to the parent" do
-          expect(child.actions).to eq([:dont_eat, :dont_sleep])
+          expect(child.actions).to eq([:nothing, :dont_eat, :dont_sleep])
         end
 
         it "does not delegate #default_action to the parent" do
@@ -457,11 +457,50 @@ describe "LWRP" do
 
         it "amends actions when they are already defined" do
           raise_if_deprecated!
-          expect(child.actions).to eq([:eat, :sleep, :drink])
+          expect(child.actions).to eq([:nothing, :eat, :sleep, :drink])
         end
       end
     end
 
+    describe "when actions is set to an array" do
+      let(:resource_class) do
+        Class.new(Chef::Resource::LWRPBase) do
+          actions [ :eat, :sleep ]
+        end
+      end
+      let(:resource) do
+        resource_class.new('blah')
+      end
+      it "actions includes those actions" do
+        expect(resource_class.actions).to eq [ :nothing, :eat, :sleep ]
+      end
+      it "allowed_actions includes those actions" do
+        expect(resource_class.allowed_actions).to eq [ :nothing, :eat, :sleep ]
+      end
+      it "resource.allowed_actions includes those actions" do
+        expect(resource.allowed_actions).to eq [ :nothing, :eat, :sleep ]
+      end
+    end
+
+    describe "when allowed_actions is set to an array" do
+      let(:resource_class) do
+        Class.new(Chef::Resource::LWRPBase) do
+          allowed_actions [ :eat, :sleep ]
+        end
+      end
+      let(:resource) do
+        resource_class.new('blah')
+      end
+      it "actions includes those actions" do
+        expect(resource_class.actions).to eq [ :nothing, :eat, :sleep ]
+      end
+      it "allowed_actions includes those actions" do
+        expect(resource_class.allowed_actions).to eq [ :nothing, :eat, :sleep ]
+      end
+      it "resource.allowed_actions includes those actions" do
+        expect(resource.allowed_actions).to eq [ :nothing, :eat, :sleep ]
+      end
+    end
   end
 
   describe "Lightweight Chef::Provider" do