Added a 'search' method to the ldap node terminus.

This makes it easy to find multiple nodes in ldap, and
was done so it can be used by puppetrun.

4 files changed, 333 insertions, 295 deletions

diff --git a/lib/puppet/indirector/ldap.rb b/lib/puppet/indirector/ldap.rb
index 695d38a95..7c3aca0da 100644
--- a/lib/puppet/indirector/ldap.rb
+++ b/lib/puppet/indirector/ldap.rb
@@ -1,21 +1,14 @@
 require 'puppet/indirector/terminus'
 
 class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
-    # We split this apart so it's easy to call multiple times with different names.
-    def entry2hash(name)
-        # We have to use 'yield' here because the LDAP::Entry objects
-        # get destroyed outside the scope of the search, strangely.
-        ldapsearch(name) { |entry| return process(name, entry) }
-    end
-
     # Perform our ldap search and process the result.
     def find(request)
-        return entry2hash(request.key) || nil
+        return ldapsearch(search_filter(request.key)) { |entry| return process(entry) } || nil
     end
 
     # Process the found entry.  We assume that we don't just want the
     # ldap object.
-    def process(name, entry)
+    def process(entry)
         raise Puppet::DevError, "The 'process' method has not been overridden for the LDAP terminus for %s" % self.name
     end
 
@@ -35,14 +28,14 @@ class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
 
     # Find the ldap node, return the class list and parent node specially,
     # and everything else in a parameter hash.
-    def ldapsearch(node)
+    def ldapsearch(filter)
         raise ArgumentError.new("You must pass a block to ldapsearch") unless block_given?
 
         found = false
         count = 0
 
         begin
-            connection.search(search_base, 2, search_filter(node), search_attributes) do |entry|
+            connection.search(search_base, 2, filter, search_attributes) do |entry|
                 found = true
                 yield entry
             end
@@ -54,7 +47,9 @@ class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
                 Puppet.warning "Retrying LDAP connection"
                 retry
             else
-                raise Puppet::Error, "LDAP Search failed: %s" % detail
+                error = Puppet::Error.new("LDAP Search failed")
+                error.set_backtrace(detail.backtrace)
+                raise error
             end
         end
 
diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 4ed053eff..2f953bbcb 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -13,6 +13,18 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         x = Puppet[:ldapclassattrs].split(/\s*,\s*/)
     end
 
+    # Separate this out so it's relatively atomic.  It's tempting to call
+    # process() instead of entry2hash() here, but it ends up being
+    # difficult to test because all exceptions get caught by ldapsearch.
+    # LAK:NOTE Unfortunately, the ldap support is too stupid to throw anything
+    # but LDAP::ResultError, even on bad connections, so we are rough handed
+    # with our error handling.
+    def name2hash(name)
+        info = nil
+        ldapsearch(search_filter(name)) { |entry| info = entry2hash(entry) }
+        return info
+    end
+
     # Look for our node in ldap.
     def find(request)
         names = [request.key]
@@ -21,19 +33,40 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         end
         names << "default"
 
-        information = nil
+        node = nil
         names.each do |name|
-            break if information = entry2hash(name)
+            break if node = process(name)
         end
-        return nil unless information
+        return nil unless node
 
-        name = request.key
+        node.name = request.key
 
-        node = Puppet::Node.new(name)
+        return node
+    end
 
-        add_to_node(node, information)
+    def process(name)
+        return nil unless info = name2hash(name)
 
-        return node
+        info2node(name, info)
+    end
+
+    # Find more than one node.  LAK:NOTE This is a bit of a clumsy API, because the 'search'
+    # method currently *requires* a key.  It seems appropriate in some cases but not others,
+    # and I don't really know how to get rid of it as a requirement but allow it when desired.
+    def search(key, options = {})
+        if classes = options[:class]
+            classes = [classes] unless classes.is_a?(Array)
+            filter = "(&(objectclass=puppetClient)(puppetclass=" + classes.join(")(puppetclass=") + "))"
+        else
+            filter = "(objectclass=puppetClient)"
+        end
+
+        infos = []
+        ldapsearch(filter) { |entry| infos << entry2hash(entry) }
+
+        return infos.collect do |info|
+            info2node(info[:name], info)
+        end
     end
 
     # The parent attribute, if we have one.
@@ -51,15 +84,15 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         Puppet[:ldapstackedattrs].split(/\s*,\s*/)
     end
 
-    # Process the found entry.  We assume that we don't just want the
-    # ldap object.
-    def process(name, entry)
+    # Convert the found entry into a simple hash.
+    def entry2hash(entry)
         result = {}
+        result[:name] = entry.dn.split(',')[0].split("=")[1]
         if pattr = parent_attribute
             if values = entry.vals(pattr)
                 if values.length > 1
                     raise Puppet::Error,
-                        "Node %s has more than one parent: %s" % [name, values.inspect]
+                        "Node entry %s specifies more than one parent: %s" % [entry.dn, values.inspect]
                 end
                 unless values.empty?
                     result[:parent] = values.shift
@@ -73,9 +106,11 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
                 values.each do |v| result[:classes] << v end
             end
         }
+        result[:classes].uniq!
 
         result[:stacked] = []
-        stacked_attributes.each { |attr|
+        stacked_params = stacked_attributes
+        stacked_params.each { |attr|
             if values = entry.vals(attr)
                 result[:stacked] = result[:stacked] + values
             end
@@ -83,17 +118,34 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         
 
         result[:parameters] = entry.to_hash.inject({}) do |hash, ary|
-            if ary[1].length == 1
-                hash[ary[0]] = ary[1].shift
-            else
-                hash[ary[0]] = ary[1]
+            unless stacked_params.include?(ary[0]) # don't add our stacked parameters to the main param list
+                if ary[1].length == 1
+                    hash[ary[0]] = ary[1].shift
+                else
+                    hash[ary[0]] = ary[1]
+                end
             end
             hash
         end
 
         result[:environment] = result[:parameters]["environment"] if result[:parameters]["environment"]
 
-        return result
+        result[:stacked_parameters] = {}
+
+        if result[:stacked]
+            result[:stacked].each do |value|
+                param = value.split('=', 2)
+                result[:stacked_parameters][param[0]] = param[1]
+            end
+        end
+
+        if result[:stacked_parameters]
+            result[:stacked_parameters].each do |param, value|
+                result[:parameters][param] = value unless result[:parameters].include?(param)
+            end
+        end
+
+        result
     end
 
     # Default to all attributes.
@@ -128,51 +180,17 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
 
     # Add our hash of ldap information to the node instance.
     def add_to_node(node, information)
-        information[:stacked_parameters] = {}
-
-        parent_info = nil
-        parent = information[:parent]
-        parents = [node.name]
-        while parent
-            if parents.include?(parent)
-                raise ArgumentError, "Found loop in LDAP node parents; %s appears twice" % parent
-            end
-            parents << parent
-            parent = find_and_merge_parent(parent, information)
-        end
-
-        if information[:stacked]
-            information[:stacked].each do |value|
-                param = value.split('=', 2)
-                information[:stacked_parameters][param[0]] = param[1]
-            end
-        end
-
-        if information[:stacked_parameters]
-            information[:stacked_parameters].each do |param, value|
-                information[:parameters][param] = value unless information[:parameters].include?(param)
-            end
-        end
-
         node.classes = information[:classes].uniq unless information[:classes].nil? or information[:classes].empty?
         node.parameters = information[:parameters] unless information[:parameters].nil? or information[:parameters].empty?
         node.environment = information[:environment] if information[:environment]
-        node.fact_merge
     end
 
     # Find information for our parent and merge it into the current info.
     def find_and_merge_parent(parent, information)
-        parent_info = nil
-        ldapsearch(parent) { |entry| parent_info = process(parent, entry) }
-
-        unless parent_info
+        unless parent_info = name2hash(parent)
             raise Puppet::Error.new("Could not find parent node '%s'" % parent)
         end
         information[:classes] += parent_info[:classes]
-        parent_info[:stacked].each do |value|
-            param = value.split('=', 2)
-            information[:stacked_parameters][param[0]] = param[1]
-        end
         parent_info[:parameters].each do |param, value|
             # Specifically test for whether it's set, so false values are handled
             # correctly.
@@ -183,4 +201,34 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
 
         parent_info[:parent]
     end
+
+    # Take a name and a hash, and return a node instance.
+    def info2node(name, info)
+        merge_parent(info) if info[:parent]
+
+        node = Puppet::Node.new(name)
+
+        add_to_node(node, info)
+
+        node.fact_merge
+
+        node
+    end
+
+    def merge_parent(info)
+        parent_info = nil
+        parent = info[:parent]
+
+        # Preload the parent array with the node name.
+        parents = [info[:name]]
+        while parent
+            if parents.include?(parent)
+                raise ArgumentError, "Found loop in LDAP node parents; %s appears twice" % parent
+            end
+            parents << parent
+            parent = find_and_merge_parent(parent, info)
+        end
+
+        return info
+    end
 end
diff --git a/spec/unit/indirector/ldap.rb b/spec/unit/indirector/ldap.rb
index 2599bcecf..cf802d856 100755
--- a/spec/unit/indirector/ldap.rb
+++ b/spec/unit/indirector/ldap.rb
@@ -28,8 +28,9 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
         @request = stub 'request', :key => "yay"
     end
 
-    it "should call the ldapsearch method with the name being searched for" do
-        @searcher.expects(:ldapsearch).with("yay")
+    it "should call the ldapsearch method with the search filter" do
+        @searcher.expects(:search_filter).with("yay").returns("yay's filter")
+        @searcher.expects(:ldapsearch).with("yay's filter")
         @searcher.find @request
     end
 
@@ -61,15 +62,6 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
         @searcher.find @request
     end
 
-    it "should use the results of the :search_filter method as the search filter" do
-        @searcher.stubs(:search_filter).with("yay").returns("yay's filter")
-        @connection.expects(:search).with do |*args|
-            args[2].should == "yay's filter"
-            true
-        end
-        @searcher.find @request
-    end
-
     it "should use depth 2 when searching" do
         @connection.expects(:search).with do |*args|
             args[1].should == 2
@@ -80,7 +72,7 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
 
     it "should call process() on the first found entry" do
         @connection.expects(:search).yields("myresult")
-        @searcher.expects(:process).with("yay", "myresult")
+        @searcher.expects(:process).with("myresult")
         @searcher.find @request
     end
 
@@ -94,7 +86,7 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
                 raise "failed"
             end
         end.yields("myresult")
-        @searcher.expects(:process).with("yay", "myresult")
+        @searcher.expects(:process).with("myresult")
 
         @searcher.find @request
     end
diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb
index 24b2dd759..03faaa557 100755
--- a/spec/unit/indirector/node/ldap.rb
+++ b/spec/unit/indirector/node/ldap.rb
@@ -5,304 +5,307 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/indirector/node/ldap'
 
 describe Puppet::Node::Ldap do
-    describe "when searching for nodes" do
+    before do
+        Puppet::Node::Facts.stubs(:terminus_class).returns :yaml
+    end
+
+    describe "when searching for a single node" do
         before :each do
             @searcher = Puppet::Node::Ldap.new
-            @entries = {}
-            entries = @entries
             
-            @connection = mock 'connection'
-            @entry = mock 'entry'
-            @connection.stubs(:search).yields(@entry)
-            @searcher.stubs(:connection).returns(@connection)
-            @searcher.stubs(:class_attributes).returns([])
-            @searcher.stubs(:parent_attribute).returns(nil)
-            @searcher.stubs(:stacked_attributes).returns([])
-            @searcher.stubs(:search_base).returns(:yay)
-            @searcher.stubs(:search_filter).returns(:filter)
-
             @name = "mynode.domain.com"
-            @node = stub 'node', :name => @name
-            @node.stubs(:fact_merge)
-            Puppet::Node.stubs(:new).with(@name).returns(@node)
+            @node = stub 'node', :name => @name, :name= => nil
+            @node.stub_everything
+
+            Puppet::Node.stubs(:new).returns(@node)
 
             @request = stub 'request', :key => @name
         end
 
-        it "should search first for the provided key" do
-            @searcher.expects(:entry2hash).with("mynode.domain.com").returns({})
-            @searcher.find(@request)
+        it "should convert the hostname into a search filter" do
+            entry = stub 'entry', :dn => 'cn=mynode.domain.com,ou=hosts,dc=madstop,dc=com', :vals => %w{}, :to_hash => {}
+            @searcher.expects(:ldapsearch).with("(&(objectclass=puppetClient)(cn=#{@name}))").yields entry
+            @searcher.name2hash(@name)
         end
 
-        it "should search for the short version of the provided key if the key looks like a hostname and no results are found for the key itself" do
-            @searcher.expects(:entry2hash).with("mynode.domain.com").returns(nil)
-            @searcher.expects(:entry2hash).with("mynode").returns({})
-            @searcher.find(@request)
+        it "should convert any found entry into a hash" do
+            entry = stub 'entry', :dn => 'cn=mynode.domain.com,ou=hosts,dc=madstop,dc=com', :vals => %w{}, :to_hash => {}
+            @searcher.expects(:ldapsearch).with("(&(objectclass=puppetClient)(cn=#{@name}))").yields entry
+            @searcher.expects(:entry2hash).with(entry).returns "myhash"
+            @searcher.name2hash(@name).should == "myhash"
         end
 
-        it "should search for default information if no information can be found for the key" do
-            @searcher.expects(:entry2hash).with("mynode.domain.com").returns(nil)
-            @searcher.expects(:entry2hash).with("mynode").returns(nil)
-            @searcher.expects(:entry2hash).with("default").returns({})
-            @searcher.find(@request)
-        end
+        # This heavily tests our entry2hash method, so we don't have to stub out the stupid entry information any more.
+        describe "when an ldap entry is found" do
+            before do
+                @entry = stub 'entry', :dn => 'cn=mynode.domain.com,ou=hosts,dc=madstop,dc=com', :vals => %w{}, :to_hash => {}
+                @searcher.stubs(:ldapsearch).yields @entry
+            end
 
-        it "should return nil if no results are found in ldap" do
-            @connection.stubs(:search)
-            @searcher.find(@request).should be_nil
-        end
+            it "should convert the entry to a hash" do
+                @searcher.entry2hash(@entry).should be_instance_of(Hash)
+            end
 
-        it "should return a node object if results are found in ldap" do
-            @entry.stubs(:to_hash).returns({})
-            @searcher.find(@request).should equal(@node)
-        end
+            it "should add the entry's common name to the hash" do
+                @searcher.entry2hash(@entry)[:name].should == "mynode.domain.com"
+            end
 
-        it "should deduplicate class values" do
-            @entry.stubs(:to_hash).returns({})
-            @searcher.stubs(:class_attributes).returns(%w{one two})
-            @entry.stubs(:vals).with("one").returns(%w{a b})
-            @entry.stubs(:vals).with("two").returns(%w{b c})
-            @node.expects(:classes=).with(%w{a b c})
-            @searcher.find(@request)
-        end
+            it "should add all of the entry's classes to the hash" do
+                @entry.stubs(:vals).with("puppetclass").returns %w{one two}
+                @searcher.entry2hash(@entry)[:classes].should == %w{one two}
+            end
 
-        it "should add any values stored in the class_attributes attributes to the node classes" do
-            @entry.stubs(:to_hash).returns({})
-            @searcher.stubs(:class_attributes).returns(%w{one two})
-            @entry.stubs(:vals).with("one").returns(%w{a b})
-            @entry.stubs(:vals).with("two").returns(%w{c d})
-            @node.expects(:classes=).with(%w{a b c d})
-            @searcher.find(@request)
+            it "should deduplicate class values" do
+                @entry.stubs(:to_hash).returns({})
+                @searcher.stubs(:class_attributes).returns(%w{one two})
+                @entry.stubs(:vals).with("one").returns(%w{a b})
+                @entry.stubs(:vals).with("two").returns(%w{b c})
+                @searcher.entry2hash(@entry)[:classes].should == %w{a b c}
+            end
+
+            it "should add the entry's environment to the hash" do
+                @entry.stubs(:to_hash).returns("environment" => %w{production})
+                @searcher.entry2hash(@entry)[:environment].should == "production"
+            end
+
+            it "should add all stacked parameters as parameters in the hash" do
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two three=four})
+                result = @searcher.entry2hash(@entry)
+                result[:parameters]["one"].should == "two"
+                result[:parameters]["three"].should == "four"
+            end
+
+            it "should not add the stacked parameter as a normal parameter" do
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two three=four})
+                @entry.stubs(:to_hash).returns("puppetvar" => %w{one=two three=four})
+                @searcher.entry2hash(@entry)[:parameters]["puppetvar"].should be_nil
+            end
+
+            it "should add all other attributes as parameters in the hash" do
+                @entry.stubs(:to_hash).returns("foo" => %w{one two})
+                @searcher.entry2hash(@entry)[:parameters]["foo"].should == %w{one two}
+            end
+
+            it "should return single-value parameters as strings, not arrays" do
+                @entry.stubs(:to_hash).returns("foo" => %w{one})
+                @searcher.entry2hash(@entry)[:parameters]["foo"].should == "one"
+            end
+
+            it "should add the parent's name if present" do
+                @entry.stubs(:vals).with("parentnode").returns(%w{foo})
+                @searcher.entry2hash(@entry)[:parent].should == "foo"
+            end
+
+            it "should fail if more than one parent is specified" do
+                @entry.stubs(:vals).with("parentnode").returns(%w{foo})
+                @searcher.entry2hash(@entry)[:parent].should == "foo"
+            end
         end
 
-        it "should add all entry attributes as node parameters" do
-            @entry.stubs(:to_hash).returns("one" => ["two"], "three" => ["four"])
-            @node.expects(:parameters=).with("one" => "two", "three" => "four")
+        it "should search first for the provided key" do
+            @searcher.expects(:name2hash).with("mynode.domain.com").returns({})
             @searcher.find(@request)
         end
 
-        it "should set the node's environment to the environment of the results" do
-            @entry.stubs(:to_hash).returns("environment" => ["test"])
-            @node.stubs(:parameters=)
-            @node.expects(:environment=).with("test")
+        it "should search for the short version of the provided key if the key looks like a hostname and no results are found for the key itself" do
+            @searcher.expects(:name2hash).with("mynode.domain.com").returns(nil)
+            @searcher.expects(:name2hash).with("mynode").returns({})
             @searcher.find(@request)
         end
 
-        it "should retain false parameter values" do
-            @entry.stubs(:to_hash).returns("one" => [false])
-            @node.expects(:parameters=).with("one" => false)
+        it "should search for default information if no information can be found for the key" do
+            @searcher.expects(:name2hash).with("mynode.domain.com").returns(nil)
+            @searcher.expects(:name2hash).with("mynode").returns(nil)
+            @searcher.expects(:name2hash).with("default").returns({})
             @searcher.find(@request)
         end
 
-        it "should turn single-value parameter value arrays into single non-arrays" do
-            @entry.stubs(:to_hash).returns("one" => ["a"])
-            @node.expects(:parameters=).with("one" => "a")
-            @searcher.find(@request)
+        it "should return nil if no results are found in ldap" do
+            @searcher.stubs(:name2hash).returns nil
+            @searcher.find(@request).should be_nil
         end
 
-        it "should keep multi-valued parametes as arrays" do
-            @entry.stubs(:to_hash).returns("one" => ["a", "b"])
-            @node.expects(:parameters=).with("one" => ["a", "b"])
-            @searcher.find(@request)
+        it "should return a node object if results are found in ldap" do
+            @searcher.stubs(:name2hash).returns({})
+            @searcher.find(@request).should equal(@node)
         end
 
-        describe "and a parent node is specified" do
+        describe "and node information is found in LDAP" do
             before do
-                @parent = mock 'parent'
-                @parent_parent = mock 'parent_parent'
-
-                @searcher.meta_def(:search_filter) do |name|
-                    return name
-                end
-                @connection.stubs(:search).with { |*args| args[2] == @name              }.yields(@entry)
-                @connection.stubs(:search).with { |*args| args[2] == 'parent'           }.yields(@parent)
-                @connection.stubs(:search).with { |*args| args[2] == 'parent_parent'    }.yields(@parent_parent)
-
-                @searcher.stubs(:parent_attribute).returns(:parent)
+                @result = {}
+                @searcher.stubs(:name2hash).returns @result
             end
 
-            it "should fail if the parent cannot be found" do
-                @connection.stubs(:search).with { |*args| args[2] == 'parent' }.returns("whatever")
-
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
-
-                proc { @searcher.find(@request) }.should raise_error(Puppet::Error)
-            end
-
-            it "should add any parent classes to the node's classes" do
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
-                @entry.stubs(:vals).with("classes").returns(%w{a b})
-
-                @parent.stubs(:to_hash).returns({})
-                @parent.stubs(:vals).with("classes").returns(%w{c d})
-                @parent.stubs(:vals).with(:parent).returns(nil)
-
-                @searcher.stubs(:class_attributes).returns(%w{classes})
+            it "should add any classes from ldap" do
+                @result[:classes] = %w[a b c d]
                 @node.expects(:classes=).with(%w{a b c d})
                 @searcher.find(@request)
             end
 
-            it "should add any parent parameters to the node's parameters" do
-                @entry.stubs(:to_hash).returns("one" => "two")
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
-
-                @parent.stubs(:to_hash).returns("three" => "four")
-                @parent.stubs(:vals).with(:parent).returns(nil)
-
+            it "should add all entry attributes as node parameters" do
+                @result[:parameters] = {"one" => "two", "three" => "four"}
                 @node.expects(:parameters=).with("one" => "two", "three" => "four")
                 @searcher.find(@request)
             end
 
-            it "should prefer node parameters over parent parameters" do
-                @entry.stubs(:to_hash).returns("one" => "two")
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
-
-                @parent.stubs(:to_hash).returns("one" => "three")
-                @parent.stubs(:vals).with(:parent).returns(nil)
+            it "should set the node's environment to the environment of the results" do
+                @result[:environment] = "test"
+                @node.expects(:environment=).with("test")
+                @searcher.find(@request)
+            end
 
-                @node.expects(:parameters=).with("one" => "two")
+            it "should retain false parameter values" do
+                @result[:parameters] = {}
+                @result[:parameters]["one"] = false
+                @node.expects(:parameters=).with("one" => false)
                 @searcher.find(@request)
             end
 
-            it "should use the parent's environment if the node has none" do
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+            describe "and a parent node is specified" do
+                before do
+                    @entry = {:classes => [], :parameters => {}}
+                    @parent = {:classes => [], :parameters => {}}
+                    @parent_parent = {:classes => [], :parameters => {}}
 
-                @parent.stubs(:to_hash).returns("environment" => ["parent"])
-                @parent.stubs(:vals).with(:parent).returns(nil)
+                    @searcher.stubs(:name2hash).with(@name).returns(@entry)
+                    @searcher.stubs(:name2hash).with('parent').returns(@parent)
+                    @searcher.stubs(:name2hash).with('parent_parent').returns(@parent_parent)
 
-                @node.stubs(:parameters=)
-                @node.expects(:environment=).with("parent")
-                @searcher.find(@request)
-            end
+                    @searcher.stubs(:parent_attribute).returns(:parent)
+                end
 
-            it "should prefer the node's environment to the parent's" do
-                @entry.stubs(:to_hash).returns("environment" => %w{child})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                it "should search for the parent node" do
+                    @entry[:parent] = "parent"
 
-                @parent.stubs(:to_hash).returns("environment" => ["parent"])
-                @parent.stubs(:vals).with(:parent).returns(nil)
+                    @searcher.expects(:name2hash).with(@name).returns @entry
+                    @searcher.expects(:name2hash).with('parent').returns @parent
 
-                @node.stubs(:parameters=)
-                @node.expects(:environment=).with("child")
-                @searcher.find(@request)
-            end
+                    @searcher.find(@request)
+                end
 
-            it "should recursively look up parent information" do
-                @entry.stubs(:to_hash).returns("one" => "two")
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                it "should fail if the parent cannot be found" do
+                    @entry[:parent] = "parent"
 
-                @parent.stubs(:to_hash).returns("three" => "four")
-                @parent.stubs(:vals).with(:parent).returns(['parent_parent'])
+                    @searcher.expects(:name2hash).with('parent').returns nil
 
-                @parent_parent.stubs(:to_hash).returns("five" => "six")
-                @parent_parent.stubs(:vals).with(:parent).returns(nil)
-                @parent_parent.stubs(:vals).with(:parent).returns(nil)
+                    proc { @searcher.find(@request) }.should raise_error(Puppet::Error)
+                end
 
-                @node.expects(:parameters=).with("one" => "two", "three" => "four", "five" => "six")
-                @searcher.find(@request)
-            end
+                it "should add any parent classes to the node's classes" do
+                    @entry[:parent] = "parent"
+                    @entry[:classes] = %w{a b}
 
-            it "should not allow loops in parent declarations" do
-                @entry.stubs(:to_hash).returns("one" => "two")
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                    @parent[:classes] = %w{c d}
 
-                @parent.stubs(:to_hash).returns("three" => "four")
-                @parent.stubs(:vals).with(:parent).returns([@name])
-                proc { @searcher.find(@request) }.should raise_error(ArgumentError)
-            end
-        end
+                    @node.expects(:classes=).with(%w{a b c d})
+                    @searcher.find(@request)
+                end
 
-        describe "and a puppet variable is specified" do
-            before do
-                @searcher.stubs(:stacked_attributes).returns(['puppetvar'])
-            end
+                it "should add any parent parameters to the node's parameters" do
+                    @entry[:parent] = "parent"
+                    @entry[:parameters]["one"] = "two"
 
-            it "should add the variable to the node parameters" do
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @entry.stubs(:to_hash).returns({})
-                @node.expects(:parameters=).with("one" => "two")
-                @searcher.find(@request)
-            end
+                    @parent[:parameters]["three"] = "four"
 
-            it "should not overwrite node parameters specified as ldap object attribute" do
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @entry.stubs(:to_hash).returns("one" => "three")
-                @node.expects(:parameters=).with("one" => "three")
-                @searcher.find(@request)
-            end
+                    @node.expects(:parameters=).with("one" => "two", "three" => "four")
+                    @searcher.find(@request)
+                end
 
-            it "should set entries without an equal sign to nil" do
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one})
-                @entry.stubs(:to_hash).returns({})
-                @node.expects(:parameters=).with("one" => nil)
-                @searcher.find(@request)
-            end
+                it "should prefer node parameters over parent parameters" do
+                    @entry[:parent] = "parent"
+                    @entry[:parameters]["one"] = "two"
 
-            it "should ignore empty entries" do
-                @entry.stubs(:vals).with("puppetvar").returns(%w{})
-                @entry.stubs(:to_hash).returns({})
-                @searcher.find(@request)
-            end
-        end
-        describe "and a puppet variable as well as a parent node are specified" do
-            before do
-                @parent = mock 'parent'
+                    @parent[:parameters]["one"] = "three"
 
-                @searcher.meta_def(:search_filter) do |name|
-                    return name
+                    @node.expects(:parameters=).with("one" => "two")
+                    @searcher.find(@request)
                 end
-                @connection.stubs(:search).with { |*args| args[2] == @name              }.yields(@entry)
-                @connection.stubs(:search).with { |*args| args[2] == 'parent'           }.yields(@parent)
 
-                @searcher.stubs(:stacked_attributes).returns(['puppetvar'])
-                @searcher.stubs(:parent_attribute).returns(:parent)
-            end
+                it "should use the parent's environment if the node has none" do
+                    @entry[:parent] = "parent"
 
-            it "should add parent node variables to the child node parameters" do
-                @parent.stubs(:to_hash).returns({})
-                @parent.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @parent.stubs(:vals).with(:parent).returns(nil)
+                    @parent[:environment] = "parent"
 
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with("puppetvar").returns(%w{})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                    @node.stubs(:parameters=)
+                    @node.expects(:environment=).with("parent")
+                    @searcher.find(@request)
+                end
 
-                @node.expects(:parameters=).with("one" => "two")
+                it "should prefer the node's environment to the parent's" do
+                    @entry[:parent] = "parent"
+                    @entry[:environment] = "child"
 
-                @searcher.find(@request)
-            end
+                    @parent[:environment] = "parent"
 
-            it "should overwrite parent node variables with child node parameters" do
-                @parent.stubs(:to_hash).returns({})
-                @parent.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @parent.stubs(:vals).with(:parent).returns(nil)
+                    @node.stubs(:parameters=)
+                    @node.expects(:environment=).with("child")
+                    @searcher.find(@request)
+                end
 
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one=three})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                it "should recursively look up parent information" do
+                    @entry[:parent] = "parent"
+                    @entry[:parameters]["one"] = "two"
 
-                @node.expects(:parameters=).with("one" => "three")
+                    @parent[:parent] = "parent_parent"
+                    @parent[:parameters]["three"] = "four"
 
-                @searcher.find(@request)
+                    @parent_parent[:parameters]["five"] = "six"
+
+                    @node.expects(:parameters=).with("one" => "two", "three" => "four", "five" => "six")
+                    @searcher.find(@request)
+                end
+
+                it "should not allow loops in parent declarations" do
+                    @entry[:parent] = "parent"
+                    @parent[:parent] = @name
+                    proc { @searcher.find(@request) }.should raise_error(ArgumentError)
+                end
             end
+        end
+    end
 
-            it "should not overwrite parent node parameters specified as ldap object attribute" do
-                @parent.stubs(:to_hash).returns("one" => "three")
-                @parent.stubs(:vals).with("puppetvar").returns(%w{})
-                @parent.stubs(:vals).with(:parent).returns(nil)
+    describe "when searching for multiple nodes" do
+        before :each do
+            @searcher = Puppet::Node::Ldap.new
+            @request = stub 'request', :key => @name
 
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+            Puppet::Node::Facts.stubs(:terminus_class).returns :yaml
+        end
 
-                @node.expects(:parameters=).with("one" => "three")
+        it "should find all nodes if no arguments are provided" do
+            @searcher.expects(:ldapsearch).with("(objectclass=puppetClient)")
+            # LAK:NOTE The search method requires an essentially bogus key.  It's
+            # an API problem that I don't really know how to fix.
+            @searcher.search "foo"
+        end
 
-                @searcher.find(@request)
+        describe "and a class is specified" do
+            it "should find all nodes that are members of that class" do
+                @searcher.expects(:ldapsearch).with("(&(objectclass=puppetClient)(puppetclass=one))")
+                @searcher.search "foo", :class => "one"
+            end
+        end
+
+        describe "multiple classes are specified" do
+            it "should find all nodes that are members of all classes" do
+                @searcher.expects(:ldapsearch).with("(&(objectclass=puppetClient)(puppetclass=one)(puppetclass=two))")
+                @searcher.search "foo", :class => ["one", "two"]
             end
+        end
+
+        it "should process each found entry" do
+            # .yields can't be used to yield multiple values :/
+            @searcher.expects(:ldapsearch).yields("one")
+            @searcher.expects(:entry2hash).with("one").returns(:name => "foo")
+            @searcher.search "foo"
+        end
 
+        it "should return a node for each processed entry" do
+            @searcher.expects(:ldapsearch).yields("one")
+            @searcher.expects(:entry2hash).with("one").returns(:name => "foo")
+            result = @searcher.search("foo")
+            result[0].should be_instance_of(Puppet::Node)
+            result[0].name.should == "foo"
         end
     end
 end