(#4576) Raise an error when a node is classified into a non-existent class

In 2.6.x, this was upgraded from "info" to "warning". This change for
Statler escalates the warning to an exception which will abort the
compile. This makes compiling fail consistently when you try to use an
undefined class from any of: node classifiers, the class keyword, and the
include function.

Paired-with: Jacob Helwig <jacob@puppetlabs.com>

3 files changed, 12 insertions, 30 deletions

diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index fdabd05c9..a891f1a11 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -20,7 +20,7 @@ class Puppet::Parser::Compiler
     puts detail.backtrace if Puppet[:trace]
     raise Puppet::Error, "#{detail} on node #{node.name}"
   ensure
-    # We get these from the environment and only cache them in a thread 
+    # We get these from the environment and only cache them in a thread
     # variable for the duration of the compilation.
     Thread.current[:known_resource_types] = nil
     Thread.current[:env_module_directories] = nil
@@ -133,12 +133,11 @@ class Puppet::Parser::Compiler
   end
 
   # Evaluate each specified class in turn.  If there are any classes we can't
-  # find, just tag the catalog and move on.  This method really just
-  # creates resource objects that point back to the classes, and then the
-  # resources are themselves evaluated later in the process.
+  # find, raise an error.  This method really just creates resource objects
+  # that point back to the classes, and then the resources are themselves
+  # evaluated later in the process.
   def evaluate_classes(classes, scope, lazy_evaluate = true)
     raise Puppet::DevError, "No source for scope passed to evaluate_classes" unless scope.source
-    found = []
     param_classes = nil
     # if we are a param class, save the classes hash
     # and transform classes to be the keys
@@ -153,20 +152,17 @@ class Puppet::Parser::Compiler
         if param_classes
           resource = klass.ensure_in_catalog(scope, param_classes[name] || {})
         else
-          found << name and next if scope.class_scope(klass)
+          next if scope.class_scope(klass)
           resource = klass.ensure_in_catalog(scope)
         end
 
         # If they've disabled lazy evaluation (which the :include function does),
         # then evaluate our resource immediately.
         resource.evaluate unless lazy_evaluate
-        found << name
       else
-        Puppet.info "Could not find class #{name} for #{node.name}"
-        @catalog.tag(name)
+        raise Puppet::Error, "Could not find class #{name} for #{node.name}"
       end
     end
-    found
   end
 
   def evaluate_relationships
diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb
index 18f9a93b6..e4b18e14b 100755
--- a/spec/unit/parser/compiler_spec.rb
+++ b/spec/unit/parser/compiler_spec.rb
@@ -576,18 +576,16 @@ describe Puppet::Parser::Compiler do
       proc { @compiler.evaluate_classes(%w{one two}, scope) }.should raise_error(Puppet::DevError)
     end
 
-    it "should tag the catalog with the name of each not-found class" do
-      @compiler.catalog.expects(:tag).with("notfound")
+    it "should raise an error if a class is not found" do
       @scope.expects(:find_hostclass).with("notfound").returns(nil)
-      @compiler.evaluate_classes(%w{notfound}, @scope)
+      lambda{ @compiler.evaluate_classes(%w{notfound}, @scope) }.should raise_error(Puppet::Error, /Could not find class/)
     end
-    # I wish it would fail
-    it "should log when it can't find class" do
+
+    it "should raise an error when it can't find class" do
       klasses = {'foo'=>nil}
       @node.classes = klasses
       @compiler.topscope.stubs(:find_hostclass).with('foo').returns(nil)
-      Puppet.expects(:info).with('Could not find class foo for testnode')
-      @compiler.compile
+      lambda{ @compiler.compile }.should raise_error(Puppet::Error, /Could not find class foo for testnode/)
     end
   end
 
@@ -714,18 +712,6 @@ describe Puppet::Parser::Compiler do
       Puppet::Parser::Resource.expects(:new).never
       @compiler.evaluate_classes(%w{MyClass}, @scope, false)
     end
-
-    it "should return the list of found classes" do
-      @compiler.catalog.stubs(:tag)
-
-      @compiler.stubs(:add_resource)
-      @scope.stubs(:find_hostclass).with("notfound").returns(nil)
-      @scope.stubs(:class_scope).with(@class)
-
-      Puppet::Parser::Resource.stubs(:new).returns(@resource)
-      @class.stubs :ensure_in_catalog
-      @compiler.evaluate_classes(%w{myclass notfound}, @scope).should == %w{myclass}
-    end
   end
 
   describe "when evaluating AST nodes with no AST nodes present" do
diff --git a/test/language/functions.rb b/test/language/functions.rb
index 3c27f2b2b..e882b68f3 100755
--- a/test/language/functions.rb
+++ b/test/language/functions.rb
@@ -447,7 +447,7 @@ class TestLangFunctions < Test::Unit::TestCase
 
     include = Puppet::Parser::Functions.function(:include)
 
-    assert_raise(Puppet::ParseError, "did not throw error on missing class") do
+    assert_raise(Puppet::Error, "did not throw error on missing class") do
       scope.function_include("nosuchclass")
     end