Merge branch 'tickets/0.24.x/1390' of git://github.com/lak/puppet into 0.24.x

2 files changed, 37 insertions, 14 deletions

diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 71d3e3b0e..01010a2af 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -14,7 +14,7 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
     end
 
     # Separate this out so it's relatively atomic.  It's tempting to call
-    # process() instead of entry2hash() here, but it ends up being
+    # process() instead of name2hash() 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
@@ -35,21 +35,14 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
 
         node = nil
         names.each do |name|
-            break if node = process(name)
-        end
-        return nil unless node
+            next unless info = name2hash(name)
 
-        node.name = request.key
+            break if node = info2node(request.key, info)
+        end
 
         return node
     end
 
-    def process(name)
-        return nil unless info = name2hash(name)
-
-        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.
diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb
index 01d148631..ed8809e73 100755
--- a/spec/unit/indirector/node/ldap.rb
+++ b/spec/unit/indirector/node/ldap.rb
@@ -136,6 +136,14 @@ describe Puppet::Node::Ldap do
                 @searcher.stubs(:name2hash).returns @result
             end
 
+            it "should create the node with the correct name, even if it was found by a different name" do
+                @searcher.expects(:name2hash).with("mynode.domain.com").returns nil
+                @searcher.expects(:name2hash).with("mynode").returns @result
+
+                Puppet::Node.expects(:new).with("mynode.domain.com").returns @node
+                @searcher.find(@request)
+            end
+
             it "should add any classes from ldap" do
                 @result[:classes] = %w[a b c d]
                 @node.expects(:classes=).with(%w{a b c d})
@@ -161,6 +169,19 @@ describe Puppet::Node::Ldap do
                 @searcher.find(@request)
             end
 
+            it "should merge the node's facts after the parameters from ldap are assigned" do
+                # Make sure we've got data to start with, so the parameters are actually set.
+                @result[:parameters] = {}
+                @result[:parameters]["one"] = "yay"
+
+                # A hackish way to enforce order.
+                set = false
+                @node.expects(:parameters=).with { |*args| set = true }
+                @node.expects(:fact_merge).with { |*args| raise "Facts were merged before parameters were set" unless set; true }
+
+                @searcher.find(@request)
+            end
+
             describe "and a parent node is specified" do
                 before do
                     @entry = {:classes => [], :parameters => {}}
@@ -304,13 +325,22 @@ describe Puppet::Node::Ldap do
             @searcher.search @request
         end
 
-        it "should return a node for each processed entry" do
-            @searcher.expects(:ldapsearch).yields("one")
-            @searcher.expects(:entry2hash).with("one").returns(:name => "foo")
+        it "should return a node for each processed entry with the name from the entry" do
+            @searcher.expects(:ldapsearch).yields("whatever")
+            @searcher.expects(:entry2hash).with("whatever").returns(:name => "foo")
             result = @searcher.search(@request)
             result[0].should be_instance_of(Puppet::Node)
             result[0].name.should == "foo"
         end
+
+        it "should merge each node's facts" do
+            node = mock 'node'
+            Puppet::Node.expects(:new).with("foo").returns node
+            node.expects(:fact_merge)
+            @searcher.stubs(:ldapsearch).yields("one")
+            @searcher.stubs(:entry2hash).with("one").returns(:name => "foo")
+            @searcher.search(@request)
+        end
     end
 end