Fixing the Node::Ldap.search method to use an indirection request.

I foolishly was just using the old-style api.

Added an integration test to catch this in the future.

3 files changed, 34 insertions, 8 deletions

diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 2f953bbcb..71d3e3b0e 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -53,8 +53,8 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
     # 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]
+    def search(request)
+        if classes = request.options[:class]
             classes = [classes] unless classes.is_a?(Array)
             filter = "(&(objectclass=puppetClient)(puppetclass=" + classes.join(")(puppetclass=") + "))"
         else
diff --git a/spec/integration/indirector/node/ldap.rb b/spec/integration/indirector/node/ldap.rb
new file mode 100755
index 000000000..34f4cb159
--- /dev/null
+++ b/spec/integration/indirector/node/ldap.rb
@@ -0,0 +1,22 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+require 'puppet/indirector/node/ldap'
+
+describe Puppet::Node::Ldap do
+    before do
+        Puppet[:node_terminus] = :ldap
+        Puppet::Node.stubs(:terminus_class).returns :ldap
+    end
+
+    after do
+        Puppet.settings.clear
+    end
+
+    it "should use a restrictive filter when searching for nodes in a class" do
+        Puppet::Node.indirection.terminus(:ldap).expects(:ldapsearch).with("(&(objectclass=puppetClient)(puppetclass=foo))")
+
+        Puppet::Node.search "eh", :class => "foo"
+    end
+end
diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb
index 03faaa557..01d148631 100755
--- a/spec/unit/indirector/node/ldap.rb
+++ b/spec/unit/indirector/node/ldap.rb
@@ -267,7 +267,8 @@ describe Puppet::Node::Ldap do
     describe "when searching for multiple nodes" do
         before :each do
             @searcher = Puppet::Node::Ldap.new
-            @request = stub 'request', :key => @name
+            @options = {}
+            @request = stub 'request', :key => "foo", :options => @options
 
             Puppet::Node::Facts.stubs(:terminus_class).returns :yaml
         end
@@ -276,20 +277,23 @@ describe Puppet::Node::Ldap 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"
+            @searcher.search @request
         end
 
         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"
+
+                @options[:class] = "one"
+                @searcher.search @request
             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"]
+                @options[:class] = %w{one two}
+                @searcher.search @request
             end
         end
 
@@ -297,13 +301,13 @@ describe Puppet::Node::Ldap 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"
+            @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")
-            result = @searcher.search("foo")
+            result = @searcher.search(@request)
             result[0].should be_instance_of(Puppet::Node)
             result[0].name.should == "foo"
         end