Fix for #2765 (--no-fqdn regression in puppetrun)

This is basically the fix suggested on the ticket, cleaned up and
ruby-ized, with tests.  The only functional modification is leaving
the default on entry2hash as --no-fqdn to preserve 0.25.1 behaviour
as the default.

Signed- ff-by: Markus Roberts <Markus@reality.com>

4 files changed, 40 insertions, 13 deletions

diff --git a/lib/puppet/application/puppetrun.rb b/lib/puppet/application/puppetrun.rb
index 41ebf9f1a..4febcf524 100644
--- a/lib/puppet/application/puppetrun.rb
+++ b/lib/puppet/application/puppetrun.rb
@@ -167,12 +167,12 @@ Puppet::Application.new(:puppetrun) do
 
         if Puppet[:node_terminus] == "ldap" and (options[:all] or @classes)
             if options[:all]
-                @hosts = Puppet::Node.search("whatever").collect { |node| node.name }
+                @hosts = Puppet::Node.search("whatever", :fqdn => options[:fqdn]).collect { |node| node.name }
                 puts "all: %s" % @hosts.join(", ")
             else
                 @hosts = []
                 @classes.each do |klass|
-                    list = Puppet::Node.search("whatever", :class => klass).collect { |node| node.name }
+                    list = Puppet::Node.search("whatever", :fqdn => options[:fqdn], :class => klass).collect { |node| node.name }
                     puts "%s: %s" % [klass, list.join(", ")]
 
                     @hosts += list
diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index dd8cebfac..954bc8d78 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -55,7 +55,7 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         end
 
         infos = []
-        ldapsearch(filter) { |entry| infos << entry2hash(entry) }
+        ldapsearch(filter) { |entry| infos << entry2hash(entry, request.options[:fqdn]) }
 
         return infos.collect do |info|
             info2node(info[:name], info)
@@ -78,9 +78,12 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
     end
 
     # Convert the found entry into a simple hash.
-    def entry2hash(entry)
+    def entry2hash(entry, fqdn = false)
         result = {}
-        result[:name] = entry.dn.split(',')[0].split("=")[1]
+
+        cn  = entry.dn[     /cn\s*=\s*([^,\s]+)/i,1]
+        dcs = entry.dn.scan(/dc\s*=\s*([^,\s]+)/i)
+        result[:name]    = fqdn ? ([cn]+dcs).join('.') : cn
         result[:parent] = get_parent_from_entry(entry) if parent_attribute
         result[:classes] = get_classes_from_entry(entry)
         result[:stacked] = get_stacked_values_from_entry(entry)
diff --git a/spec/unit/application/puppetrun.rb b/spec/unit/application/puppetrun.rb
index d44b406e9..26811f0db 100755
--- a/spec/unit/application/puppetrun.rb
+++ b/spec/unit/application/puppetrun.rb
@@ -144,11 +144,21 @@ describe "puppetrun" do
                 Puppet.stubs(:[]).with(:node_terminus).returns("ldap")
             end
 
+            it "should pass the fqdn option to search" do
+                @puppetrun.options.stubs(:[]).with(:fqdn).returns(:something)
+                @puppetrun.options.stubs(:[]).with(:all).returns(true)
+                @puppetrun.stubs(:puts)
+
+                Puppet::Node.expects(:search).with("whatever",:fqdn => :something).returns([])
+
+                @puppetrun.run_setup
+            end
+
             it "should search for all nodes if --all" do
                 @puppetrun.options.stubs(:[]).with(:all).returns(true)
                 @puppetrun.stubs(:puts)
 
-                Puppet::Node.expects(:search).with("whatever").returns([])
+                Puppet::Node.expects(:search).with("whatever",:fqdn => nil).returns([])
 
                 @puppetrun.run_setup
             end
@@ -158,7 +168,7 @@ describe "puppetrun" do
                 @puppetrun.stubs(:puts)
                 @puppetrun.classes = ['class']
 
-                Puppet::Node.expects(:search).with("whatever", :class => "class").returns([])
+                Puppet::Node.expects(:search).with("whatever", :class => "class", :fqdn => nil).returns([])
 
                 @puppetrun.run_setup
             end
diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb
index e25bc36c4..953a8c2b2 100755
--- a/spec/unit/indirector/node/ldap.rb
+++ b/spec/unit/indirector/node/ldap.rb
@@ -38,7 +38,7 @@ describe Puppet::Node::Ldap do
         # 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 => {}
+                @entry = stub 'entry', :dn => 'cn=mynode,ou=hosts,dc=madstop,dc=com', :vals => %w{}, :to_hash => {}
                 @searcher.stubs(:ldapsearch).yields @entry
             end
 
@@ -46,8 +46,12 @@ describe Puppet::Node::Ldap do
                 @searcher.entry2hash(@entry).should be_instance_of(Hash)
             end
 
-            it "should add the entry's common name to the hash" do
-                @searcher.entry2hash(@entry)[:name].should == "mynode.domain.com"
+            it "should add the entry's common name to the hash if fqdn if false" do
+                @searcher.entry2hash(@entry,fqdn = false)[:name].should == "mynode"
+            end
+
+            it "should add the entry's fqdn name to the hash if fqdn if true" do
+                @searcher.entry2hash(@entry,fqdn = true)[:name].should == "mynode.madstop.com"
             end
 
             it "should add all of the entry's classes to the hash" do
@@ -341,13 +345,13 @@ describe Puppet::Node::Ldap do
         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.expects(:entry2hash).with("one",nil).returns(:name => "foo")
             @searcher.search @request
         end
 
         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")
+            @searcher.expects(:entry2hash).with("whatever",nil).returns(:name => "foo")
             result = @searcher.search(@request)
             result[0].should be_instance_of(Puppet::Node)
             result[0].name.should == "foo"
@@ -358,7 +362,17 @@ describe Puppet::Node::Ldap do
             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.stubs(:entry2hash).with("one",nil).returns(:name => "foo")
+            @searcher.search(@request)
+        end
+
+        it "should pass the request's fqdn option to entry2hash" do
+            node = mock 'node'
+            @options[:fqdn] = :hello
+            Puppet::Node.stubs(:new).with("foo").returns node
+            node.stubs(:fact_merge)
+            @searcher.stubs(:ldapsearch).yields("one")
+            @searcher.expects(:entry2hash).with("one",:hello).returns(:name => "foo")
             @searcher.search(@request)
         end
     end