Raise an error when including a recipe from an unreachable cookbook

Fixes CHEF-4367.

When attempting to load a recipe belonging to a cookbook that is not in
the run_list or any dependencies of cookbooks in the run_list, chef
will now produce an error like this:

    Chef::Exceptions::MissingCookbookDependency
    -------------------------------------------
    Recipe `ancient::aliens` is not in the run_list, and cookbook 'ancient'
    is not a dependency of any cookbook in the run_list.  To load thisrecipe,
    first add a dependency on cookbook 'ancient' in the cookbook you're
    including it from in that cookbook's metadata.

This error will occur when chef-solo users use `include_recipe` without
specifying the dependency in metadata; prior to this patch, chef would
typically fail reading an undefined attribute, which commonly would
result in a NoMethodError for nil.

9 files changed, 76 insertions, 12 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cb9320ab3c..1b27fc25b4 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ## Unreleased
 
+* Including a recipe from a cookbook not in the dependency graph raises
+  a MissingCookbookDependency exception. Fixes CHEF-4367.
 * Improves syntax check speed for Ruby 1.9+, especially when using bundler.
 * Send X-Remote-Request-Id header in order to be able to correlate actions during a single run.
 * Fix for CHEF-5048.
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index f976263188..8df49b9303 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -76,6 +76,7 @@ class Chef
     class CookbookNotFoundInRepo < ArgumentError; end
     class RecipeNotFound < ArgumentError; end
     class AttributeNotFound < RuntimeError; end
+    class MissingCookbookDependency < StandardError; end
     class InvalidCommandOption < RuntimeError; end
     class CommandTimeout < RuntimeError; end
     class RequestedUIDUnavailable < RuntimeError; end
diff --git a/lib/chef/run_context.rb b/lib/chef/run_context.rb
index 05a954ad15..b66e3e54c7 100644
--- a/lib/chef/run_context.rb
+++ b/lib/chef/run_context.rb
@@ -77,13 +77,15 @@ class Chef
       @events = events
 
       @node.run_context = self
+
+      @cookbook_compiler = nil
     end
 
     # Triggers the compile phase of the chef run. Implemented by
     # Chef::RunContext::CookbookCompiler
     def load(run_list_expansion)
-      compiler = CookbookCompiler.new(self, run_list_expansion, events)
-      compiler.compile
+      @cookbook_compiler = CookbookCompiler.new(self, run_list_expansion, events)
+      @cookbook_compiler.compile
     end
 
     # Adds an immediate notification to the
@@ -141,6 +143,17 @@ class Chef
       Chef::Log.debug("Loading Recipe #{recipe_name} via include_recipe")
 
       cookbook_name, recipe_short_name = Chef::Recipe.parse_recipe_name(recipe_name)
+
+      if unreachable_cookbook?(cookbook_name) # CHEF-4367
+        raise(Exceptions::MissingCookbookDependency,<<-ERROR_MESSAGE)
+Recipe `#{recipe_name}` is not in the run_list, and cookbook '#{cookbook_name}'
+is not a dependency of any cookbook in the run_list.  To load this recipe,
+first add a dependency on cookbook '#{cookbook_name}' in the cookbook you're
+including it from in that cookbook's metadata.
+ERROR_MESSAGE
+      end
+
+
       if loaded_fully_qualified_recipe?(cookbook_name, recipe_short_name)
         Chef::Log.debug("I am not loading #{recipe_name}, because I have already seen it.")
         false
@@ -228,6 +241,12 @@ class Chef
       cookbook.has_cookbook_file_for_node?(node, cb_file_name)
     end
 
+    # Delegates to CookbookCompiler#unreachable_cookbook?
+    # Used to raise an error when attempting to load a recipe belonging to a
+    # cookbook that is not in the dependency graph. See also: CHEF-4367
+    def unreachable_cookbook?(cookbook_name)
+      @cookbook_compiler.unreachable_cookbook?(cookbook_name)
+    end
 
     private
 
diff --git a/lib/chef/run_context/cookbook_compiler.rb b/lib/chef/run_context/cookbook_compiler.rb
index 0a05061152..abe5afa7ae 100644
--- a/lib/chef/run_context/cookbook_compiler.rb
+++ b/lib/chef/run_context/cookbook_compiler.rb
@@ -16,6 +16,7 @@
 # limitations under the License.
 #
 
+require 'set'
 require 'chef/log'
 require 'chef/recipe'
 require 'chef/resource/lwrp_base'
@@ -149,6 +150,17 @@ class Chef
         @events.recipe_load_complete
       end
 
+      # Whether or not a cookbook is reachable from the set of cookbook given
+      # by the run_list plus those cookbooks' dependencies.
+      def unreachable_cookbook?(cookbook_name)
+        !reachable_cookbooks.include?(cookbook_name)
+      end
+
+      # All cookbooks in the dependency graph, returned as a Set.
+      def reachable_cookbooks
+        @reachable_cookbooks ||= Set.new(cookbook_order)
+      end
+
       private
 
       def load_attributes_from_cookbook(cookbook_name)
diff --git a/spec/integration/solo/solo_spec.rb b/spec/integration/solo/solo_spec.rb
index cd4678f94d..f688a16430 100644
--- a/spec/integration/solo/solo_spec.rb
+++ b/spec/integration/solo/solo_spec.rb
@@ -42,6 +42,25 @@ E
 
   end
 
+  when_the_repository "has a cookbook with an undeclared dependency" do
+    file 'cookbooks/x/metadata.rb', 'version "1.0.0"'
+    file 'cookbooks/x/recipes/default.rb', 'include_recipe "ancient::aliens"'
+
+    file 'cookbooks/ancient/metadata.rb', 'version "1.0.0"'
+    file 'cookbooks/ancient/recipes/aliens.rb', 'print "it was aliens"'
+
+    it "should exit with an error" do
+      file 'config/solo.rb', <<EOM
+cookbook_path "#{path_to('cookbooks')}"
+file_cache_path "#{path_to('config/cache')}"
+EOM
+      result = shell_out("ruby bin/chef-solo -c \"#{path_to('config/solo.rb')}\" -o 'x::default' -l debug", :cwd => chef_dir)
+      result.exitstatus.should == 1
+      result.stdout.should include("Chef::Exceptions::MissingCookbookDependency")
+    end
+  end
+
+
   when_the_repository "has a cookbook with a recipe with sleep" do
     directory 'logs'
     file 'logs/runs.log', ''
diff --git a/spec/unit/cookbook_spec.rb b/spec/unit/cookbook_spec.rb
index ca4f4adc08..9bcea97d98 100644
--- a/spec/unit/cookbook_spec.rb
+++ b/spec/unit/cookbook_spec.rb
@@ -68,16 +68,6 @@ describe Chef::CookbookVersion do
     @cookbook.preferred_filename(@node, :files, 'a-filename', 'the-checksum').should be_nil
   end
 
-  it "should allow you to include a fully-qualified recipe using the DSL" do
-    # DSL method include_recipe allows multiple arguments, so extract the first
-    @node.should_receive(:loaded_recipe).with(:openldap, "gigantor")
-    recipe = @run_context.include_recipe("openldap::gigantor").first
-
-    recipe.recipe_name.should == "gigantor"
-    recipe.cookbook_name.should == :openldap
-    @run_context.resource_collection[0].name.should == "blanket"
-  end
-
   it "should raise an ArgumentException if you try to load a bad recipe name" do
     lambda { @cookbook.load_recipe("doesnt_exist", @node) }.should raise_error(ArgumentError)
   end
diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb
index b0cd04b245..2bdf470143 100644
--- a/spec/unit/recipe_spec.rb
+++ b/spec/unit/recipe_spec.rb
@@ -339,6 +339,7 @@ describe Chef::Recipe do
   describe "include_recipe" do
     it "should evaluate another recipe with include_recipe" do
       node.should_receive(:loaded_recipe).with(:openldap, "gigantor")
+      run_context.stub(:unreachable_cookbook?).with(:openldap).and_return(false)
       run_context.include_recipe "openldap::gigantor"
       res = run_context.resource_collection.resources(:cat => "blanket")
       res.name.should eql("blanket")
@@ -347,6 +348,7 @@ describe Chef::Recipe do
 
     it "should load the default recipe for a cookbook if include_recipe is called without a ::" do
       node.should_receive(:loaded_recipe).with(:openldap, "default")
+      run_context.stub(:unreachable_cookbook?).with(:openldap).and_return(false)
       run_context.include_recipe "openldap"
       res = run_context.resource_collection.resources(:cat => "blanket")
       res.name.should eql("blanket")
@@ -355,12 +357,14 @@ describe Chef::Recipe do
 
     it "should store that it has seen a recipe in the run_context" do
       node.should_receive(:loaded_recipe).with(:openldap, "default")
+      run_context.stub(:unreachable_cookbook?).with(:openldap).and_return(false)
       run_context.include_recipe "openldap"
       run_context.loaded_recipe?("openldap").should be_true
     end
 
     it "should not include the same recipe twice" do
       node.should_receive(:loaded_recipe).with(:openldap, "default").exactly(:once)
+      run_context.stub(:unreachable_cookbook?).with(:openldap).and_return(false)
       cookbook_collection[:openldap].should_receive(:load_recipe).with("default", run_context)
       recipe.include_recipe "openldap"
       cookbook_collection[:openldap].should_not_receive(:load_recipe).with("default", run_context)
diff --git a/spec/unit/run_context/cookbook_compiler_spec.rb b/spec/unit/run_context/cookbook_compiler_spec.rb
index 52f4772206..5c50c3dd4b 100644
--- a/spec/unit/run_context/cookbook_compiler_spec.rb
+++ b/spec/unit/run_context/cookbook_compiler_spec.rb
@@ -170,5 +170,17 @@ describe Chef::RunContext::CookbookCompiler do
                                          :"circular-dep1",
                                          :"test-with-circular-deps"]
     end
+
+    it "determines if a cookbook is in the list of cookbooks reachable by dependency" do
+      node.run_list("test-with-deps::default", "test-with-deps::server")
+      compiler.cookbook_order.should == [:dependency1, :dependency2, :"test-with-deps"]
+      compiler.unreachable_cookbook?(:dependency1).should be_false
+      compiler.unreachable_cookbook?(:dependency2).should be_false
+      compiler.unreachable_cookbook?(:'test-with-deps').should be_false
+      compiler.unreachable_cookbook?(:'circular-dep1').should be_true
+      compiler.unreachable_cookbook?(:'circular-dep2').should be_true
+    end
+
+
   end
 end
diff --git a/spec/unit/run_context_spec.rb b/spec/unit/run_context_spec.rb
index 39b8a8a50d..f885f49770 100644
--- a/spec/unit/run_context_spec.rb
+++ b/spec/unit/run_context_spec.rb
@@ -79,6 +79,11 @@ describe Chef::RunContext do
       @node.include_attribute("test::george")
     end
 
+    it "raises an error when attempting to include_recipe from a cookbook not reachable by run list or dependencies" do
+      lambda do
+        @run_context.include_recipe("ancient::aliens")
+      end.should raise_error(Chef::Exceptions::MissingCookbookDependency)
+    end
 
   end