Fix node[:cookbooks] attribute

closes #8817

Note that if any calling code winds up seeing this error message:

```
NoMethodError: undefined method `set_cookbook_attribute' for nil:NilClass
```

That means that the cookbook_collection was set before the node was
set on the run_context.  That wouldn't be a bug in core chef, that must
be fixed in the caller to reverse the order of operations.

Since I only made the positional arguments to the run_context constructor
optional in Chef-15.0 though I don't expect this breaks any existing code
written in the past month or two, but if anything crops up in the future,
consider this a definitive statement that the caller must reverse the
order of their operations and this error being thrown is a feature not
a bug to be fixed.

(The fact that we silently aborted rather than threw a NoMethodError on
NilClass meant that we shipped this defect -- sometimes defensive
programming can be overly defensive and swallow real errors).

Signed-off-by: Lamont Granquist <lamont@scriptkiddie.org>

5 files changed, 34 insertions, 9 deletions

diff --git a/lib/chef/node.rb b/lib/chef/node.rb
index ec20fbee86..1e5d3a8d59 100644
--- a/lib/chef/node.rb
+++ b/lib/chef/node.rb
@@ -87,8 +87,6 @@ class Chef
     # after the run_context has been set on the node, go through the cookbook_collection
     # and setup the node[:cookbooks] attribute so that it is published in the node object
     def set_cookbook_attribute
-      return unless run_context.cookbook_collection
-
       run_context.cookbook_collection.each do |cookbook_name, cookbook|
         automatic_attrs[:cookbooks][cookbook_name][:version] = cookbook.version
       end
diff --git a/lib/chef/policy_builder/expand_node_object.rb b/lib/chef/policy_builder/expand_node_object.rb
index 4afb4d7d60..bb5e2e199b 100644
--- a/lib/chef/policy_builder/expand_node_object.rb
+++ b/lib/chef/policy_builder/expand_node_object.rb
@@ -75,7 +75,6 @@ class Chef
       #
       def setup_run_context(specific_recipes = nil, run_context = nil)
         run_context ||= Chef::RunContext.new
-
         run_context.events = events
         run_context.node = node
 
@@ -93,6 +92,7 @@ class Chef
 
         cookbook_collection.validate!
         cookbook_collection.install_gems(events)
+
         run_context.cookbook_collection = cookbook_collection
 
         # TODO: move this into the cookbook_compilation_start hook
diff --git a/lib/chef/policy_builder/policyfile.rb b/lib/chef/policy_builder/policyfile.rb
index 70a2e44635..7eb9de042e 100644
--- a/lib/chef/policy_builder/policyfile.rb
+++ b/lib/chef/policy_builder/policyfile.rb
@@ -177,16 +177,17 @@ class Chef
       #
       # @return [Chef::RunContext]
       def setup_run_context(specific_recipes = nil, run_context = nil)
+        run_context ||= Chef::RunContext.new
+        run_context.node = node
+        run_context.events = events
+
         Chef::Cookbook::FileVendor.fetch_from_remote(api_service)
         sync_cookbooks
         cookbook_collection = Chef::CookbookCollection.new(cookbooks_to_sync)
         cookbook_collection.validate!
         cookbook_collection.install_gems(events)
 
-        run_context ||= Chef::RunContext.new
-        run_context.node = node
         run_context.cookbook_collection = cookbook_collection
-        run_context.events = events
 
         setup_chef_class(run_context)
 
diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb
index 7a5924a3c7..d849679680 100644
--- a/lib/chef/run_context.rb
+++ b/lib/chef/run_context.rb
@@ -66,7 +66,7 @@ class Chef
     #
     # @return [Chef::CookbookCollection]
     #
-    attr_accessor :cookbook_collection
+    attr_reader :cookbook_collection
 
     #
     # Resource Definitions for this run. Populated when the files in
@@ -188,11 +188,11 @@ class Chef
     # @param events [EventDispatch::Dispatcher] The event dispatcher for this
     #   run.
     #
-    def initialize(node = nil, cookbook_collection = {}, events = nil, logger = nil)
+    def initialize(node = nil, cookbook_collection = nil, events = nil, logger = nil)
       @events = events
       @logger = logger || Chef::Log.with_child
-      @cookbook_collection = cookbook_collection
       self.node = node if node
+      self.cookbook_collection = cookbook_collection if cookbook_collection
       @definitions = {}
       @loaded_recipes_hash = {}
       @loaded_attributes_hash = {}
@@ -205,6 +205,10 @@ class Chef
     def node=(node)
       @node = node
       node.run_context = self
+    end
+
+    def cookbook_collection=(cookbook_collection)
+      @cookbook_collection = cookbook_collection
       node.set_cookbook_attribute
     end
 
diff --git a/spec/integration/client/client_spec.rb b/spec/integration/client/client_spec.rb
index 68cfd015ab..7c0cbc6a63 100644
--- a/spec/integration/client/client_spec.rb
+++ b/spec/integration/client/client_spec.rb
@@ -369,6 +369,28 @@ describe "chef-client" do
     end
   end
 
+  when_the_repository "has a cookbook that outputs some node attributes" do
+    before do
+      file "cookbooks/x/recipes/default.rb", <<~'EOM'
+        puts "COOKBOOKS: #{node[:cookbooks]}"
+      EOM
+      file "cookbooks/x/metadata.rb", <<~EOM
+        name 'x'
+        version '0.0.1'
+      EOM
+      file "config/client.rb", <<~EOM
+        local_mode true
+        cookbook_path "#{path_to("cookbooks")}"
+      EOM
+    end
+
+    it "should fail the chef client run" do
+      result = shell_out("#{chef_client} -c \"#{path_to("config/client.rb")}\" -o 'x::default' --no-fork", cwd: chef_dir)
+      result.error!
+      expect(result.stdout).to include('COOKBOOKS: {"x"=>{"version"=>"0.0.1"}}')
+    end
+  end
+
   when_the_repository "has a cookbook that should fail chef_version checks" do
     before do
       file "cookbooks/x/recipes/default.rb", ""