Adding the last tests for the ldap node terminus. I managed

to forget the tests around the main find() method.

4 files changed, 211 insertions, 15 deletions

diff --git a/lib/puppet/indirector/ldap.rb b/lib/puppet/indirector/ldap.rb
index 63848d10a..fb883def6 100644
--- a/lib/puppet/indirector/ldap.rb
+++ b/lib/puppet/indirector/ldap.rb
@@ -5,7 +5,7 @@ class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
     def find(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(entry) }
+        ldapsearch(name) { |entry| return process(name, entry) }
 
         # Return nil if we haven't found something.
         return nil
@@ -13,7 +13,7 @@ class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
 
     # Process the found entry.  We assume that we don't just want the
     # ldap object.
-    def process(entry)
+    def process(name, entry)
         raise Puppet::DevError, "The 'process' method has not been overridden for the LDAP terminus for %s" % self.name
     end
 
diff --git a/lib/puppet/indirector/ldap/node.rb b/lib/puppet/indirector/ldap/node.rb
index 187c74b94..6f35b575c 100644
--- a/lib/puppet/indirector/ldap/node.rb
+++ b/lib/puppet/indirector/ldap/node.rb
@@ -10,23 +10,34 @@ class Puppet::Indirector::Ldap::Node < Puppet::Indirector::Ldap
 
     # Look for our node in ldap.
     def find(name)
-        unless ary = ldapsearch(name)
-            return nil
-        end
-        parent, classes, parameters = ary
+        return nil unless information = super
+        node = Puppet::Node.new(name)
 
+        parent_info = nil
+        parent = information[:parent]
+        parents = [name]
         while parent
-            parent, tmpclasses, tmpparams = ldapsearch(parent)
-            classes += tmpclasses if tmpclasses
-            tmpparams.each do |param, value|
+            if parents.include?(parent)
+                raise ArgumentError, "Found loop in LDAP node parents; %s appears twice" % parent
+            end
+            parents << parent
+            ldapsearch(parent) do |entry|
+                parent_info = process(parent, entry)
+            end
+            information[:classes] += parent_info[:classes]
+            parent_info[:parameters].each do |param, value|
                 # Specifically test for whether it's set, so false values are handled
                 # correctly.
-                parameters[param] = value unless parameters.include?(param)
+                information[:parameters][param] = value unless information[:parameters].include?(param)
             end
+
+            parent = parent_info[:parent]
         end
 
-        node = Puppet::Node.new(name, :classes => classes, :source => "ldap", :parameters => parameters)
+        node.classes = information[:classes].uniq unless information[:classes].empty?
+        node.parameters = information[:parameters] unless information[:parameters].empty?
         node.fact_merge
+
         return node
     end
 
@@ -41,8 +52,37 @@ class Puppet::Indirector::Ldap::Node < Puppet::Indirector::Ldap
 
     # Process the found entry.  We assume that we don't just want the
     # ldap object.
-    def process(entry)
-        raise Puppet::DevError, "The 'process' method has not been overridden for the LDAP terminus for %s" % self.name
+    def process(name, entry)
+        result = {}
+        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]
+                end
+                unless values.empty?
+                    result[:parent] = values.shift
+                end
+            end
+        end
+
+        result[:classes] = []
+        class_attributes.each { |attr|
+            if values = entry.vals(attr)
+                values.each do |v| result[:classes] << v end
+            end
+        }
+
+        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]
+            end
+            hash
+        end
+
+        return result
     end
 
     # Default to all attributes.
diff --git a/spec/unit/indirector/ldap.rb b/spec/unit/indirector/ldap.rb
index a936936bc..fe9408986 100755
--- a/spec/unit/indirector/ldap.rb
+++ b/spec/unit/indirector/ldap.rb
@@ -78,7 +78,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("myresult")
+        @searcher.expects(:process).with("yay", "myresult")
         @searcher.find "yay"
     end
 
@@ -92,7 +92,7 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
                 raise "failed"
             end
         end.yields("myresult")
-        @searcher.expects(:process).with("myresult")
+        @searcher.expects(:process).with("yay", "myresult")
 
         @searcher.find "yay"
     end
diff --git a/spec/unit/indirector/ldap/node.rb b/spec/unit/indirector/ldap/node.rb
index edaf77e92..37953db19 100755
--- a/spec/unit/indirector/ldap/node.rb
+++ b/spec/unit/indirector/ldap/node.rb
@@ -4,7 +4,163 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 
 require 'puppet/indirector/ldap/node'
 
+module LdapNodeSearching
+    def setup
+        @searcher = Puppet::Indirector::Ldap::Node.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(:search_base).returns(:yay)
+        @searcher.stubs(:search_filter).returns(:filter)
+
+        @node = mock 'node'
+        @node.stubs(:fact_merge)
+        @name = "mynode"
+        Puppet::Node.stubs(:new).with(@name).returns(@node)
+    end
+end
+
 describe Puppet::Indirector::Ldap::Node, " when searching for nodes" do
+    include LdapNodeSearching
+
+    it "should return nil if no results are found in ldap" do
+        @connection.stubs(:search)
+        @searcher.find("mynode").should be_nil
+    end
+
+    it "should return a node object if results are found in ldap" do
+        @entry.stubs(:to_hash).returns({})
+        @searcher.find("mynode").should equal(@node)
+    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("mynode")
+    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("mynode")
+    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")
+        @searcher.find("mynode")
+    end
+
+    it "should retain false parameter values" do
+        @entry.stubs(:to_hash).returns("one" => [false])
+        @node.expects(:parameters=).with("one" => false)
+        @searcher.find("mynode")
+    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("mynode")
+    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("mynode")
+    end
+end
+
+describe Puppet::Indirector::Ldap::Node, " when a parent node exists" do
+    include LdapNodeSearching
+
+    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)
+    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("one").returns(%w{a b})
+
+        @parent.stubs(:to_hash).returns({})
+        @parent.stubs(:vals).with("one").returns(%w{c d})
+        @parent.stubs(:vals).with(:parent).returns(nil)
+
+        @searcher.stubs(:class_attributes).returns(%w{one})
+        @node.expects(:classes=).with(%w{a b c d})
+        @searcher.find("mynode")
+    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)
+
+        @node.expects(:parameters=).with("one" => "two", "three" => "four")
+        @searcher.find("mynode")
+    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)
+
+        @node.expects(:parameters=).with("one" => "two")
+        @searcher.find("mynode")
+    end
+
+    it "should recursively look up parent information" 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(['parent_parent'])
+
+        @parent_parent.stubs(:to_hash).returns("five" => "six")
+        @parent_parent.stubs(:vals).with(:parent).returns(nil)
+        @parent_parent.stubs(:vals).with(:parent).returns(nil)
+
+        @node.expects(:parameters=).with("one" => "two", "three" => "four", "five" => "six")
+        @searcher.find("mynode")
+    end
+
+    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.stubs(:to_hash).returns("three" => "four")
+        @parent.stubs(:vals).with(:parent).returns([@name])
+        proc { @searcher.find("mynode") }.should raise_error(ArgumentError)
+    end
+end
+
+describe Puppet::Indirector::Ldap::Node, " when developing the search query" do
     before do
         @searcher = Puppet::Indirector::Ldap::Node.new
     end