Merge branch 'CHEF-4983'

6 files changed, 96 insertions, 26 deletions

diff --git a/lib/chef/client.rb b/lib/chef/client.rb
index 3b50874db9..5fe2c07d7e 100644
--- a/lib/chef/client.rb
+++ b/lib/chef/client.rb
@@ -226,14 +226,26 @@ class Chef
       raise Exceptions::ChildConvergeError, message
     end
 
+    # Instantiates a Chef::Node object, possibly loading the node's prior state
+    # when using chef-client. Delegates to policy_builder
+    #
+    #
+    # === Returns
+    # Chef::Node:: The node object for this chef run
     def load_node
       policy_builder.load_node
       @node = policy_builder.node
     end
 
+    # Mutates the `node` object to prepare it for the chef run. Delegates to
+    # policy_builder
+    #
+    # === Returns
+    # Chef::Node:: The updated node object
     def build_node
       policy_builder.build_node
       @run_status = Chef::RunStatus.new(node, events)
+      node
     end
 
     def setup_run_context
@@ -325,6 +337,19 @@ class Chef
       raise
     end
 
+    # Expands the run list. Delegates to the policy_builder.
+    #
+    # Normally this does not need to be called from here, it will be called by
+    # build_node. This is provided so external users (like the chefspec
+    # project) can inject custom behavior into the run process.
+    #
+    # === Returns
+    # RunListExpansion: A RunListExpansion or API compatible object.
+    def expanded_run_list
+      policy_builder.expand_run_list
+    end
+
+
     def do_windows_admin_check
       if Chef::Platform.windows?
         Chef::Log.debug("Checking for administrator privileges....")
diff --git a/lib/chef/policy_builder/expand_node_object.rb b/lib/chef/policy_builder/expand_node_object.rb
index ea01533a92..11a667aef9 100644
--- a/lib/chef/policy_builder/expand_node_object.rb
+++ b/lib/chef/policy_builder/expand_node_object.rb
@@ -121,43 +121,44 @@ class Chef
 
         setup_run_list_override
 
-        @run_list_expansion = expand_run_list
-
-        # @run_list_expansion is a RunListExpansion.
-        #
-        # Convert @expanded_run_list, which is an
-        # Array of Hashes of the form
-        #   {:name => NAME, :version_constraint => Chef::VersionConstraint },
-        # into @expanded_run_list_with_versions, an
-        # Array of Strings of the form
-        #   "#{NAME}@#{VERSION}"
-        @expanded_run_list_with_versions = @run_list_expansion.recipes.with_version_constraints_strings
+        expand_run_list
 
         Chef::Log.info("Run List is [#{node.run_list}]")
         Chef::Log.info("Run List expands to [#{@expanded_run_list_with_versions.join(', ')}]")
 
-
         events.node_load_completed(node, @expanded_run_list_with_versions, Chef::Config)
 
         node
       end
 
-      ########################################
-      # Internal public API
-      ########################################
-
+      # Expands the node's run list. Stores the run_list_expansion object for later use.
       def expand_run_list
-        if Chef::Config[:solo]
+        @run_list_expansion = if Chef::Config[:solo]
           node.expand!('disk')
         else
           node.expand!('server')
         end
+
+        # @run_list_expansion is a RunListExpansion.
+        #
+        # Convert @expanded_run_list, which is an
+        # Array of Hashes of the form
+        #   {:name => NAME, :version_constraint => Chef::VersionConstraint },
+        # into @expanded_run_list_with_versions, an
+        # Array of Strings of the form
+        #   "#{NAME}@#{VERSION}"
+        @expanded_run_list_with_versions = @run_list_expansion.recipes.with_version_constraints_strings
+        @run_list_expansion
       rescue Exception => e
         # TODO: wrap/munge exception with useful error output.
         events.run_list_expand_failed(node, e)
         raise
       end
 
+      ########################################
+      # Internal public API
+      ########################################
+
       # Sync_cookbooks eagerly loads all files except files and
       # templates.  It returns the cookbook_hash -- the return result
       # from /environments/#{node.chef_environment}/cookbook_versions,
diff --git a/lib/chef/policy_builder/policyfile.rb b/lib/chef/policy_builder/policyfile.rb
index 3c89f810fa..4373c36c86 100644
--- a/lib/chef/policy_builder/policyfile.rb
+++ b/lib/chef/policy_builder/policyfile.rb
@@ -51,7 +51,7 @@ class Chef
 
       class PolicyfileError < StandardError; end
 
-      RunListExpansionIsh = Struct.new(:recipes)
+      RunListExpansionIsh = Struct.new(:recipes, :roles)
 
       attr_reader :events
       attr_reader :node
@@ -100,10 +100,14 @@ class Chef
         nil
       end
 
-      # Policyfile gives you the run_list already expanded, no expansion is
-      # performed here.
+      # Policyfile gives you the run_list already expanded, but users of this
+      # class may expect to get a run_list expansion compatible object by
+      # calling this method.
+      #
+      # === Returns
+      # RunListExpansionIsh:: A RunListExpansion duck type
       def run_list_expansion
-        nil
+        run_list_expansion_ish
       end
 
       ## PolicyBuilder API ##
@@ -134,6 +138,7 @@ class Chef
 
         node.consume_external_attrs(ohai_data, json_attribs)
 
+        expand_run_list
         apply_policyfile_attributes
 
         Chef::Log.info("Run List is [#{run_list}]")
@@ -162,6 +167,13 @@ class Chef
         run_context
       end
 
+      def expand_run_list
+        node.run_list(run_list)
+        node.automatic_attrs[:roles] = []
+        node.automatic_attrs[:recipes] = run_list_expansion_ish.recipes
+        run_list_expansion_ish
+      end
+
       ## Internal Public API ##
 
       def sync_cookbooks
@@ -190,13 +202,10 @@ class Chef
           cookbook, recipe = parse_recipe_spec(recipe_spec)
           "#{cookbook}::#{recipe}"
         end
-        RunListExpansionIsh.new(recipes)
+        RunListExpansionIsh.new(recipes, [])
       end
 
       def apply_policyfile_attributes
-        node.run_list(run_list)
-        node.automatic_attrs[:roles] = []
-        node.automatic_attrs[:recipes] = run_list_expansion_ish.recipes
         node.attributes.role_default = policy["default_attributes"]
         node.attributes.role_override = policy["override_attributes"]
       end
diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb
index 6b7dd36930..f33f8d494c 100644
--- a/spec/unit/client_spec.rb
+++ b/spec/unit/client_spec.rb
@@ -333,7 +333,9 @@ shared_examples_for Chef::Client do
       @node[:recipes].should be_nil
 
       @client.policy_builder.stub!(:node).and_return(@node)
-      @client.policy_builder.build_node
+
+      # chefspec and possibly others use the return value of this method
+      @client.build_node.should == @node
 
       # check post-conditions.
       @node[:roles].should_not be_nil
diff --git a/spec/unit/policy_builder/expand_node_object_spec.rb b/spec/unit/policy_builder/expand_node_object_spec.rb
index b452f98c80..bca48b0914 100644
--- a/spec/unit/policy_builder/expand_node_object_spec.rb
+++ b/spec/unit/policy_builder/expand_node_object_spec.rb
@@ -51,6 +51,10 @@ describe Chef::PolicyBuilder::ExpandNodeObject do
       expect(policy_builder).to respond_to(:run_context)
     end
 
+    it "implements an expand_run_list method" do
+      expect(policy_builder).to respond_to(:expand_run_list)
+    end
+
     describe "loading the node" do
 
       context "on chef-solo" do
@@ -112,6 +116,27 @@ describe Chef::PolicyBuilder::ExpandNodeObject do
 
   end
 
+  context "once the node has been loaded" do
+    let(:node) do
+      node = Chef::Node.new
+      node.name(node_name)
+      node.run_list(["recipe[a::default]", "recipe[b::server]"])
+      node
+    end
+
+    before do
+      Chef::Node.should_receive(:find_or_create).with(node_name).and_return(node)
+      policy_builder.load_node
+    end
+
+    it "expands the run_list" do
+      expect(policy_builder.expand_run_list).to be_a(Chef::RunList::RunListExpansion)
+      expect(policy_builder.run_list_expansion).to be_a(Chef::RunList::RunListExpansion)
+      expect(policy_builder.run_list_expansion.recipes).to eq(["a::default", "b::server"])
+    end
+
+  end
+
   describe "building the node" do
 
     let(:configured_environment) { nil }
diff --git a/spec/unit/policy_builder/policyfile_spec.rb b/spec/unit/policy_builder/policyfile_spec.rb
index 7e83dce804..aeda1af773 100644
--- a/spec/unit/policy_builder/policyfile_spec.rb
+++ b/spec/unit/policy_builder/policyfile_spec.rb
@@ -234,6 +234,14 @@ describe Chef::PolicyBuilder::Policyfile do
         expect(policy_builder.run_list_expansion_ish.recipes).to eq(["example1::default", "example2::server"])
       end
 
+      it "implements #expand_run_list in a manner compatible with ExpandNodeObject" do
+        Chef::Node.should_receive(:find_or_create).with(node_name).and_return(node)
+        policy_builder.load_node
+        expect(policy_builder.expand_run_list).to respond_to(:recipes)
+        expect(policy_builder.expand_run_list.recipes).to eq(["example1::default", "example2::server"])
+        expect(policy_builder.expand_run_list.roles).to eq([])
+      end
+
 
       describe "validating the Policyfile.lock" do