Rewrite for testability

Testing the methods inside of the function is a complete pain.  This
moves the code out to use the PuppetX pattern and adds some basic unit
tests that validate the logic.

Gemfile

@@ -4,10 +4,14 @@ group :test do
   gem "rake"
   gem "puppet", ENV['PUPPET_VERSION'] || '~> 3.7.0'
   gem "rspec", '< 3.2.0'
+  gem 'rspec-core'
+  gem 'rspec-mocks'
+  gem 'rspec-expectations'
   gem "rspec-puppet", :git => 'https://github.com/rodjek/rspec-puppet.git'
   gem "puppetlabs_spec_helper"
   gem "metadata-json-lint"
   gem "rspec-puppet-facts"
+  gem "net-ldap"
 end
 
 group :development do

Rakefile

@@ -35,7 +35,7 @@ PuppetSyntax.exclude_paths = exclude_paths
 
 desc "Run acceptance tests"
 RSpec::Core::RakeTask.new(:acceptance) do |t|
-  t.pattern = 'spec/acceptance'
+  t.pattern = 'spec/**/*.rb'
 end
 
 desc "Populate CONTRIBUTORS file"

lib/puppet/parser/functions/ldapquery.rb

@@ -1,75 +1,19 @@
+require 'puppet_x/ldapquery'
+
 begin
   require 'net/ldap'
 rescue
-  Puppet.warn("Missing net/ldap gem for ldapquery() function")
+  Puppet.warn("Missing net/ldap gem required for ldapquery() function")
 end
 
-Puppet::Parser::Functions.newfunction(:ldapquery, :type => :rvalue) do |args|
+Puppet::Parser::Functions.newfunction(:ldapquery,
+                                      :type => :rvalue) do |args|
 
-  Puppet.debug(args.size)
-
-  filter = args[0]
-  attributes = args[1]
-
-
-  host = Puppet[:ldapserver]
-  port = Puppet[:ldapport]
-  base = Puppet[:ldapbase]
-
-  user     = Puppet[:ldapuser]
-  password = Puppet[:ldappassword]
-
-  tls = Puppet[:ldaptls]
-  ca_file = "#{Puppet[:confdir]}/ldap_ca.pem"
-
-  conf = {
-    :host => host,
-    :port => port,
-  }
-
-  if user != '' and password != ''
-    conf[:auth] = {
-      :method   => :simple,
-      :username => user,
-      :password => password,
-    }
+  if args.size > 2
+    raise Puppet::ParseError, "Too many arguments received in ldapquery()"
   end
 
-  if tls
-    conf[:encryption] = {
-      :method      => :simple_tls,
-      :tls_options => { :ca_file => ca_file }
-    }
-  end
+  filter, attributes = args
 
-  Puppet.debug(conf)
-  Puppet.debug("Searching ldap base #{base} using #{filter} for #{attributes}")
-
-  ldap = Net::LDAP.new(conf)
-  filter = Net::LDAP::Filter.construct(filter)
-
-  data = []
-  ldap.search( :base => base, :filter => filter, :attributes => attributes) do |entry|
-    entry_data = {}
-    entry.each do |attribute, values|
-
-      attr = attribute.to_s
-
-      if values.is_a? Array and values.size > 1
-
-        entry_data[attr] = []
-
-        values.each do |v|
-          entry_data[attr] << v
-        end
-      elsif values.is_a? Array and values.size == 1
-        entry_data[attr] = values[0]
-      else
-        entry_data[attr] = values
-      end
-    end
-    data << entry_data
-  end
-  return data
+  return PuppetX::LDAPquery.new(filter, attributes).results
 end
-

lib/puppet_x/ldapquery.rb

@@ -0,0 +1,127 @@
+# Class: PuppetX::LDAPquery
+#
+
+module PuppetX
+  class LDAPquery
+    attr_reader :content
+
+    def initialize(
+      filter,
+      attributes=[],
+      base=Puppet[:ldapbase]
+    )
+      @filter = filter
+      @attributes = attributes
+      @base = base
+    end
+
+    def get_config
+      # Load the configuration variables from Puppet
+      required_vars = [
+        :ldapserver,
+        :ldapport,
+      ]
+
+      required_vars.each {|r|
+        unless Puppet[r]
+          raise Puppet::ParseError, "Missing required setting '#{r.to_s}' in puppet.conf"
+        end
+      }
+
+      host = Puppet[:ldapserver]
+      port = Puppet[:ldapport]
+
+      if Puppet[:ldapuser] and Puppet[:ldappassword]
+        user     = Puppet[:ldapuser]
+        password = Puppet[:ldappassword]
+      end
+
+      tls = Puppet[:ldaptls]
+      ca_file = "#{Puppet[:confdir]}/ldap_ca.pem"
+
+      conf = {
+        :host => host,
+        :port => port,
+      }
+
+      if user != '' and password != ''
+        conf[:auth] = {
+          :method   => :simple,
+          :username => user,
+          :password => password,
+        }
+      end
+
+      if tls
+        conf[:encryption] = {
+          :method      => :simple_tls,
+          :tls_options => { :ca_file => ca_file }
+        }
+      end
+
+      Puppet.debug(conf)
+      return conf
+    end
+
+    def get_entries()
+      # Query the LDAP server for attributes using the filter
+      #
+      # Returns: An array of Net::LDAP::Entry objects
+      ldapfilter = @filter
+      attributes = @attributes
+      base = @base
+
+      conf = self.get_config()
+
+      Puppet.debug("Searching ldap base #{base} using #{@filter} for #{@attributes}")
+
+      ldap = Net::LDAP.new(conf)
+      ldapfilter = Net::LDAP::Filter.construct(@filter)
+
+      entries = []
+
+      begin
+        ldap.search(:base => base,
+                    :filter => ldapfilter,
+                    :attributes => attributes,
+                    :time => 10) do |entry|
+          entries << entry
+        end
+        Puppet.debug(entries)
+        return entries
+      rescue
+        return []
+      end
+    end
+
+    def parse_entries
+      data = []
+      entries = get_entries()
+      entries.each do |entry|
+        entry_data = {}
+        entry.each do |attribute, values|
+
+          attr = attribute.to_s
+
+          if values.is_a? Array and values.size > 1
+            entry_data[attr] = []
+
+            values.each do |v|
+              entry_data[attr] << v.chomp
+            end
+          elsif values.is_a? Array and values.size == 1
+            entry_data[attr] = values[0].chomp
+          else
+            entry_data[attr] = values.chomp
+          end
+        end
+        data << entry_data
+      end
+      return data
+    end
+
+    def results
+      parse_entries
+    end
+  end
+end

spec/fixtures/entries_multivalue.obj

@@ -0,0 +1,4 @@
+[Iu:Net::LDAP::Entry
¯dn: uid=zach,ou=users,dc=puppetlabs,dc=com
+sshpublickey:: c3NoLXJzYSBBQUFBQi4uLjE9PSB1c2VyQHNvbWV3aGVyZQo=
+sshpublickey:: c3NoLXJzYSBBQUFBQi4uLjI9PSB1c2VyQHNvbWV3aGVyZWVsc2UK
+:ET

spec/fixtures/entries_objectClass.obj

@@ -0,0 +1,8 @@
+[Iu:Net::LDAP::Entry
Ádn: uid=zach,ou=users,dc=puppetlabs,dc=com
+objectclass: posixAccount
+objectclass: shadowAccount
+objectclass: inetOrgPerson
+objectclass: puppetPerson
+objectclass: ldapPublicKey
+objectclass: top
+:ET

spec/fixtures/entries_single.obj

@@ -0,0 +1,3 @@
+[Iu:Net::LDAP::Entry:dn: uid=zach,ou=users,dc=puppetlabs,dc=com
+uid: zach
+:ET

spec/spec_helper.rb

@@ -0,0 +1,10 @@
+require 'puppetlabs_spec_helper/module_spec_helper'
+require 'rspec-puppet'
+
+fixture_path = File.expand_path(File.join(__FILE__, '..', 'fixtures'))
+
+RSpec.configure do |c|
+  c.module_path = File.join(fixture_path, 'modules')
+  c.manifest_dir = File.join(fixture_path, 'manifests')
+  c.mock_framework = :rspec
+end

spec/unit/ldapquery_spec.rb

@@ -0,0 +1,77 @@
+require 'spec_helper'
+require 'puppet_x/ldapquery'
+require 'net/ldap'
+
+describe 'PuppetX::LDAPquery' do
+  describe 'results' do
+    let (:conf) { {
+        :host => 'ldap.example.com',
+        :port => 9009,
+    } }
+
+    let (:base) { 'dc=example,dc=com' }
+
+    it "should fail with no filter" do
+      filter = ''
+      attributes = ['uid']
+      expect { PuppetX::LDAPquery.new(filter, attributes).results }.to raise_error
+    end
+
+    it "should not fail when using defaults in puppet.conf" do
+      filter = '(uid=zach)'
+      attributes = ['uid']
+      l = PuppetX::LDAPquery.new(filter, attributes, base)
+      expect { l.get_config }.to_not raise_error
+    end
+
+    it 'should return the desired results' do
+      filter = '(uid=zach)'
+      attributes = ['uid']
+
+      wanted = [{"dn"=>"uid=zach,ou=users,dc=puppetlabs,dc=com", "uid"=>"zach"}]
+      entries = Marshal.load(File.read("spec/fixtures/entries_single.obj"))
+
+      l = PuppetX::LDAPquery.new(filter, attributes, base)
+
+      expect(l).to receive(:get_entries).and_return(entries)
+      expect(l.results).to match(wanted)
+    end
+
+    context "a multivalued attribute is requested" do
+      it 'should return the attribute values as an array to the attribute' do
+        filter = '(uid=zach)'
+        attributes = ['objectClass']
+
+        wanted = [{"dn"=>"uid=zach,ou=users,dc=puppetlabs,dc=com",
+                  "objectclass"=> [
+                    "posixAccount",
+                    "shadowAccount",
+                    "inetOrgPerson",
+                    "puppetPerson",
+                    "ldapPublicKey",
+                    "top"]}]
+
+        entries = Marshal.load(File.read("spec/fixtures/entries_objectClass.obj"))
+
+        l = PuppetX::LDAPquery.new(filter, attributes, base)
+        expect(l).to receive(:get_entries).and_return(entries)
+        expect(l.results).to match(wanted)
+      end
+      it 'should return the attributes without new lines' do
+        filter = '(uid=zach)'
+        attributes = ['sshPublicKey']
+
+        wanted = [{"dn"=>"uid=zach,ou=users,dc=puppetlabs,dc=com",
+                  "sshpublickey"=>
+        ["ssh-rsa AAAAB...1== user@somewhere",
+        "ssh-rsa AAAAB...2== user@somewhereelse"]}]
+
+        entries = Marshal.load(File.read("spec/fixtures/entries_multivalue.obj"))
+
+        l = PuppetX::LDAPquery.new(filter, attributes, base)
+        expect(l).to receive(:get_entries).and_return(entries)
+        expect(l.results).to match(wanted)
+      end
+    end
+  end
+end