(#5045) External node classifiers should be able to specify params for classes

It facilitates the support for param classes from the ENC. It adds
support for classes to be passed as a hash to the evaluate_classes
method. If a hash of classes is specified, it also evaluates duplicates.
I also had to convert the hash to an array for tags to be applied
correctly.

Reviewed-by: Matt Robinson

2 files changed, 73 insertions, 3 deletions

diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index c60e1d4fb..3134e03d9 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -139,12 +139,23 @@ class Puppet::Parser::Compiler
   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
+    if classes.class == Hash
+      param_classes = classes
+      classes = classes.keys
+    end
     classes.each do |name|
       # If we can find the class, then make a resource that will evaluate it.
       if klass = scope.find_hostclass(name)
-        found << name and next if scope.class_scope(klass)
 
-        resource = klass.ensure_in_catalog(scope)
+        if param_classes
+          resource = klass.ensure_in_catalog(scope, param_classes[name] || {})
+        else
+          found << name and next if scope.class_scope(klass) and !param_classes
+          resource = klass.ensure_in_catalog(scope)
+        end
 
         # If they've disabled lazy evaluation (which the :include function does),
         # then evaluate our resource immediately.
@@ -432,7 +443,11 @@ class Puppet::Parser::Compiler
     @resources = []
 
     # Make sure any external node classes are in our class list
-    @catalog.add_class(*@node.classes)
+    if @node.classes.class == Hash
+      @catalog.add_class(*@node.classes.keys)
+    else
+      @catalog.add_class(*@node.classes)
+    end
   end
 
   # Set the node's parameters into the top-scope as variables.
diff --git a/spec/unit/parser/compiler_spec.rb b/spec/unit/parser/compiler_spec.rb
index 95f3853e2..c1e7c48c2 100755
--- a/spec/unit/parser/compiler_spec.rb
+++ b/spec/unit/parser/compiler_spec.rb
@@ -109,6 +109,15 @@ describe Puppet::Parser::Compiler do
       compiler.classlist.should include("bar")
     end
 
+    it "should transform node class hashes into a class list" do
+      node = Puppet::Node.new("mynode")
+      node.classes = {'foo'=>{'one'=>'1'}, 'bar'=>{'two'=>'2'}}
+      compiler = Puppet::Parser::Compiler.new(node)
+
+      compiler.classlist.should include("foo")
+      compiler.classlist.should include("bar")
+    end
+
     it "should add a 'main' stage to the catalog" do
       @compiler.catalog.resource(:stage, :main).should be_instance_of(Puppet::Parser::Resource)
     end
@@ -185,6 +194,14 @@ describe Puppet::Parser::Compiler do
       @compiler.class.publicize_methods(:evaluate_node_classes) { @compiler.evaluate_node_classes }
     end
 
+    it "should evaluate any parameterized classes named in the node" do
+      classes = {'foo'=>{'1'=>'one'}, 'bar'=>{'2'=>'two'}}
+      @node.stubs(:classes).returns(classes)
+      @compiler.expects(:evaluate_classes).with(classes, @compiler.topscope)
+      @compiler.compile
+    end
+
+
     it "should evaluate the main class if it exists" do
       compile_stub(:evaluate_main)
       main_class = @known_resource_types.add Puppet::Resource::Type.new(:hostclass, "")
@@ -566,6 +583,14 @@ describe Puppet::Parser::Compiler do
       @scope.expects(:find_hostclass).with("notfound").returns(nil)
       @compiler.evaluate_classes(%w{notfound}, @scope)
     end
+    # I wish it would fail
+    it "should log 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
+    end
   end
 
   describe "when evaluating found classes" do
@@ -586,6 +611,28 @@ describe Puppet::Parser::Compiler do
       @compiler.evaluate_classes(%w{myclass}, @scope)
     end
 
+    # test that ensure_in_catalog is called for existing classes
+    it "should ensure each node class hash is in catalog" do
+      klasses = {'foo'=>{'1'=>'one'}, 'bar'=>{'2'=>'two'}}
+      @node.classes = klasses
+      klasses.each do |name, params|
+        klass = Puppet::Resource::Type.new(:hostclass, name)
+        @compiler.topscope.stubs(:find_hostclass).with(name).returns(klass)
+        klass.expects(:ensure_in_catalog).with(@compiler.topscope, params)
+      end
+      @compiler.compile
+    end
+
+    it "should ensure class is in catalog without params" do
+      @node.classes = klasses = {'foo'=>nil}
+      foo = Puppet::Resource::Type.new(:hostclass, 'foo')
+      @compiler.topscope.stubs(:find_hostclass).with('foo').returns(foo)
+      foo.expects(:ensure_in_catalog).with(@compiler.topscope, {})
+      @compiler.compile
+    end
+
+
+
     it "should not evaluate the resources created for found classes unless asked" do
       @compiler.catalog.stubs(:tag)
 
@@ -620,6 +667,14 @@ describe Puppet::Parser::Compiler do
       @compiler.evaluate_classes(%w{myclass}, @scope, false)
     end
 
+    it "should not skip param classes that have already been evaluated" do
+      @scope.stubs(:class_scope).with(@class).returns("something")
+      @node.classes = klasses = {'foo'=>nil}
+      foo = Puppet::Resource::Type.new(:hostclass, 'foo')
+      @compiler.topscope.stubs(:find_hostclass).with('foo').returns(foo)
+      foo.expects(:ensure_in_catalog).with(@compiler.topscope, {})
+      @compiler.compile
+    end
     it "should skip classes previously evaluated with different capitalization" do
       @compiler.catalog.stubs(:tag)
       @scope.stubs(:find_hostclass).with("MyClass").returns(@class)