+1, with a comment below. On Dec 17, 2009, at 8:02 PM, Markus Roberts wrote:
> 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 <[email protected]> > > Signed-off-by: Markus Roberts <[email protected]> > --- > lib/puppet/application/puppetrun.rb | 4 ++-- > lib/puppet/indirector/node/ldap.rb | 9 ++++++--- > spec/unit/application/puppetrun.rb | 14 ++++++++++++-- > spec/unit/indirector/node/ldap.rb | 26 ++++++++++++++++++++------ > 4 files changed, 40 insertions(+), 13 deletions(-) > > diff --git a/lib/puppet/application/puppetrun.rb b/lib/puppet/ > application/puppetrun.rb > index 2dbd803..b5b273d 100644 > --- a/lib/puppet/application/puppetrun.rb > +++ b/lib/puppet/application/puppetrun.rb > @@ -176,12 +176,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 dd8cebf..954bc8d 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 d44b406..26811f0 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([]) This kind of call, especially, tends to be more resilient to changes when done something like: Puppet::Node.expects(:search).with { |name, args| args[:class] == "class" }.returns nil Basically, the goal is to only test the bits whose behaviour you actually care about, and leave the rest unspecified. This way, if you add options later you don't have to go back and modify this test again. Probably not worth recutting the patch, but worth keeping in mind, especially when modifying existing tests. > > @puppetrun.run_setup > end > diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/ > indirector/node/ldap.rb > index e25bc36..953a8c2 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 > -- > 1.6.4 > > -- > > You received this message because you are subscribed to the Google > Groups "Puppet Developers" group. > To post to this group, send email to [email protected]. > To unsubscribe from this group, send email to > [email protected] > . > For more options, visit this group at > http://groups.google.com/group/puppet-dev?hl=en > . > > -- I wanna hang a map of the world in my house. Then I'm gonna put pins into all the locations that I've traveled to. But first, I'm gonna have to travel to the top two corners of the map so it won't fall down. -- Mitch Hedberg --------------------------------------------------------------------- Luke Kanies | http://reductivelabs.com | http://madstop.com -- You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
