Merge branch '0.24.x'

Conflicts:

	CHANGELOG
	spec/unit/node/catalog.rb
	spec/unit/type/package.rb
	spec/unit/type/schedule.rb
	spec/unit/type/service.rb
	spec/unit/util/settings.rb

14 files changed, 223 insertions, 153 deletions

diff --git a/lib/puppet.rb b/lib/puppet.rb
index cde25721e..e33f30ced 100644
--- a/lib/puppet.rb
+++ b/lib/puppet.rb
@@ -117,49 +117,6 @@ module Puppet
     # Load all of the configuration parameters.
     require 'puppet/defaults'
 
-    # Prints the contents of a config file with the available config elements, or it
-    # prints a single value of a config element.
-    def self.genconfig
-        if Puppet[:configprint] != ""
-            val = Puppet[:configprint]
-            if val == "all"
-                hash = {}
-                Puppet.settings.each do |name, obj|
-                    val = obj.value
-                    case val
-                    when true, false, "": val = val.inspect
-                    end
-                    hash[name] = val
-                end
-                hash.sort { |a,b| a[0].to_s <=> b[0].to_s }.each do |name, val|
-                    puts "%s = %s" % [name, val]
-                end
-            elsif val =~ /,/
-                val.split(/\s*,\s*/).sort.each do |v|
-                    if Puppet.settings.include?(v)
-                        puts "%s = %s" % [v, Puppet[v]]
-                    else
-                        puts "invalid parameter: %s" % v
-                        exit(1)
-                    end
-                end
-            else
-                val.split(/\s*,\s*/).sort.each do |v|
-                    if Puppet.settings.include?(v)
-                        puts Puppet[val]
-                    else
-                        puts "invalid parameter: %s" % v
-                        exit(1)
-                    end
-                end
-            end
-            exit(0)
-        end
-        if Puppet[:genconfig]
-            puts Puppet.settings.to_config
-            exit(0)
-        end
-    end
 
     def self.genmanifest
         if Puppet[:genmanifest]
diff --git a/lib/puppet/indirector/ldap.rb b/lib/puppet/indirector/ldap.rb
index 695d38a95..7485bd932 100644
--- a/lib/puppet/indirector/ldap.rb
+++ b/lib/puppet/indirector/ldap.rb
@@ -1,21 +1,15 @@
 require 'puppet/indirector/terminus'
+require 'puppet/util/ldap/connection'
 
 class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
-    # We split this apart so it's easy to call multiple times with different names.
-    def entry2hash(name)
-        # We have to use 'yield' here because the LDAP::Entry objects
-        # get destroyed outside the scope of the search, strangely.
-        ldapsearch(name) { |entry| return process(name, entry) }
-    end
-
     # Perform our ldap search and process the result.
     def find(request)
-        return entry2hash(request.key) || nil
+        return ldapsearch(search_filter(request.key)) { |entry| return process(entry) } || nil
     end
 
     # Process the found entry.  We assume that we don't just want the
     # ldap object.
-    def process(name, entry)
+    def process(entry)
         raise Puppet::DevError, "The 'process' method has not been overridden for the LDAP terminus for %s" % self.name
     end
 
@@ -35,14 +29,14 @@ class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
 
     # Find the ldap node, return the class list and parent node specially,
     # and everything else in a parameter hash.
-    def ldapsearch(node)
+    def ldapsearch(filter)
         raise ArgumentError.new("You must pass a block to ldapsearch") unless block_given?
 
         found = false
         count = 0
 
         begin
-            connection.search(search_base, 2, search_filter(node), search_attributes) do |entry|
+            connection.search(search_base, 2, filter, search_attributes) do |entry|
                 found = true
                 yield entry
             end
@@ -54,15 +48,15 @@ class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
                 Puppet.warning "Retrying LDAP connection"
                 retry
             else
-                raise Puppet::Error, "LDAP Search failed: %s" % detail
+                error = Puppet::Error.new("LDAP Search failed")
+                error.set_backtrace(detail.backtrace)
+                raise error
             end
         end
 
         return found
     end
 
-    private
-
     # Create an ldap connection.
     def connection
         unless defined? @connection and @connection
@@ -70,19 +64,11 @@ class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
                 raise Puppet::Error, "Could not set up LDAP Connection: Missing ruby/ldap libraries"
             end
             begin
-                if Puppet[:ldapssl]
-                    @connection = LDAP::SSLConn.new(Puppet[:ldapserver], Puppet[:ldapport])
-                elsif Puppet[:ldaptls]
-                    @connection = LDAP::SSLConn.new(
-                        Puppet[:ldapserver], Puppet[:ldapport], true
-                    )
-                else
-                    @connection = LDAP::Conn.new(Puppet[:ldapserver], Puppet[:ldapport])
-                end
-                @connection.set_option(LDAP::LDAP_OPT_PROTOCOL_VERSION, 3)
-                @connection.set_option(LDAP::LDAP_OPT_REFERRALS, LDAP::LDAP_OPT_ON)
-                @connection.simple_bind(Puppet[:ldapuser], Puppet[:ldappassword])
+                conn = Puppet::Util::Ldap::Connection.instance
+                conn.start
+                @connection = conn.connection
             rescue => detail
+                puts detail.backtrace if Puppet[:trace]
                 raise Puppet::Error, "Could not connect to LDAP: %s" % detail
             end
         end
diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 4ed053eff..71d3e3b0e 100644
--- a/lib/puppet/indirector/node/ldap.rb
+++ b/lib/puppet/indirector/node/ldap.rb
@@ -13,6 +13,18 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         x = Puppet[:ldapclassattrs].split(/\s*,\s*/)
     end
 
+    # Separate this out so it's relatively atomic.  It's tempting to call
+    # process() instead of entry2hash() 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
+    # with our error handling.
+    def name2hash(name)
+        info = nil
+        ldapsearch(search_filter(name)) { |entry| info = entry2hash(entry) }
+        return info
+    end
+
     # Look for our node in ldap.
     def find(request)
         names = [request.key]
@@ -21,19 +33,40 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         end
         names << "default"
 
-        information = nil
+        node = nil
         names.each do |name|
-            break if information = entry2hash(name)
+            break if node = process(name)
         end
-        return nil unless information
+        return nil unless node
 
-        name = request.key
+        node.name = request.key
 
-        node = Puppet::Node.new(name)
+        return node
+    end
 
-        add_to_node(node, information)
+    def process(name)
+        return nil unless info = name2hash(name)
 
-        return node
+        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.
+    def search(request)
+        if classes = request.options[:class]
+            classes = [classes] unless classes.is_a?(Array)
+            filter = "(&(objectclass=puppetClient)(puppetclass=" + classes.join(")(puppetclass=") + "))"
+        else
+            filter = "(objectclass=puppetClient)"
+        end
+
+        infos = []
+        ldapsearch(filter) { |entry| infos << entry2hash(entry) }
+
+        return infos.collect do |info|
+            info2node(info[:name], info)
+        end
     end
 
     # The parent attribute, if we have one.
@@ -51,15 +84,15 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         Puppet[:ldapstackedattrs].split(/\s*,\s*/)
     end
 
-    # Process the found entry.  We assume that we don't just want the
-    # ldap object.
-    def process(name, entry)
+    # Convert the found entry into a simple hash.
+    def entry2hash(entry)
         result = {}
+        result[:name] = entry.dn.split(',')[0].split("=")[1]
         if pattr = parent_attribute
             if values = entry.vals(pattr)
                 if values.length > 1
                     raise Puppet::Error,
-                        "Node %s has more than one parent: %s" % [name, values.inspect]
+                        "Node entry %s specifies more than one parent: %s" % [entry.dn, values.inspect]
                 end
                 unless values.empty?
                     result[:parent] = values.shift
@@ -73,9 +106,11 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
                 values.each do |v| result[:classes] << v end
             end
         }
+        result[:classes].uniq!
 
         result[:stacked] = []
-        stacked_attributes.each { |attr|
+        stacked_params = stacked_attributes
+        stacked_params.each { |attr|
             if values = entry.vals(attr)
                 result[:stacked] = result[:stacked] + values
             end
@@ -83,17 +118,34 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
         
 
         result[:parameters] = entry.to_hash.inject({}) do |hash, ary|
-            if ary[1].length == 1
-                hash[ary[0]] = ary[1].shift
-            else
-                hash[ary[0]] = ary[1]
+            unless stacked_params.include?(ary[0]) # don't add our stacked parameters to the main param list
+                if ary[1].length == 1
+                    hash[ary[0]] = ary[1].shift
+                else
+                    hash[ary[0]] = ary[1]
+                end
             end
             hash
         end
 
         result[:environment] = result[:parameters]["environment"] if result[:parameters]["environment"]
 
-        return result
+        result[:stacked_parameters] = {}
+
+        if result[:stacked]
+            result[:stacked].each do |value|
+                param = value.split('=', 2)
+                result[:stacked_parameters][param[0]] = param[1]
+            end
+        end
+
+        if result[:stacked_parameters]
+            result[:stacked_parameters].each do |param, value|
+                result[:parameters][param] = value unless result[:parameters].include?(param)
+            end
+        end
+
+        result
     end
 
     # Default to all attributes.
@@ -128,51 +180,17 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
 
     # Add our hash of ldap information to the node instance.
     def add_to_node(node, information)
-        information[:stacked_parameters] = {}
-
-        parent_info = nil
-        parent = information[:parent]
-        parents = [node.name]
-        while parent
-            if parents.include?(parent)
-                raise ArgumentError, "Found loop in LDAP node parents; %s appears twice" % parent
-            end
-            parents << parent
-            parent = find_and_merge_parent(parent, information)
-        end
-
-        if information[:stacked]
-            information[:stacked].each do |value|
-                param = value.split('=', 2)
-                information[:stacked_parameters][param[0]] = param[1]
-            end
-        end
-
-        if information[:stacked_parameters]
-            information[:stacked_parameters].each do |param, value|
-                information[:parameters][param] = value unless information[:parameters].include?(param)
-            end
-        end
-
         node.classes = information[:classes].uniq unless information[:classes].nil? or information[:classes].empty?
         node.parameters = information[:parameters] unless information[:parameters].nil? or information[:parameters].empty?
         node.environment = information[:environment] if information[:environment]
-        node.fact_merge
     end
 
     # Find information for our parent and merge it into the current info.
     def find_and_merge_parent(parent, information)
-        parent_info = nil
-        ldapsearch(parent) { |entry| parent_info = process(parent, entry) }
-
-        unless parent_info
+        unless parent_info = name2hash(parent)
             raise Puppet::Error.new("Could not find parent node '%s'" % parent)
         end
         information[:classes] += parent_info[:classes]
-        parent_info[:stacked].each do |value|
-            param = value.split('=', 2)
-            information[:stacked_parameters][param[0]] = param[1]
-        end
         parent_info[:parameters].each do |param, value|
             # Specifically test for whether it's set, so false values are handled
             # correctly.
@@ -183,4 +201,34 @@ class Puppet::Node::Ldap < Puppet::Indirector::Ldap
 
         parent_info[:parent]
     end
+
+    # Take a name and a hash, and return a node instance.
+    def info2node(name, info)
+        merge_parent(info) if info[:parent]
+
+        node = Puppet::Node.new(name)
+
+        add_to_node(node, info)
+
+        node.fact_merge
+
+        node
+    end
+
+    def merge_parent(info)
+        parent_info = nil
+        parent = info[:parent]
+
+        # Preload the parent array with the node name.
+        parents = [info[:name]]
+        while parent
+            if parents.include?(parent)
+                raise ArgumentError, "Found loop in LDAP node parents; %s appears twice" % parent
+            end
+            parents << parent
+            parent = find_and_merge_parent(parent, info)
+        end
+
+        return info
+    end
 end
diff --git a/lib/puppet/module.rb b/lib/puppet/module.rb
index b86931664..544d94ea9 100644
--- a/lib/puppet/module.rb
+++ b/lib/puppet/module.rb
@@ -63,6 +63,10 @@ class Puppet::Module
             return template
         end
 
+        # If we can find the template in :templatedir, we return that.
+        td_file = File.join(Puppet.settings.value(:templatedir, environment), template)
+        return td_file if File.exists?(td_file)
+          
         path, file = split_path(template)
 
         # Because templates don't have an assumed template name, like manifests do,
@@ -76,7 +80,7 @@ class Puppet::Module
         if mod
             return mod.template(file)
         else
-            return File.join(Puppet.settings.value(:templatedir, environment), template)
+            return td_file # Return this anyway, since we're going to fail.
         end
     end
 
diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb
index a2b6499bb..26eff52a0 100644
--- a/lib/puppet/network/client/master.rb
+++ b/lib/puppet/network/client/master.rb
@@ -26,6 +26,20 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
         
         down = Puppet[:downcasefacts]
 
+        Facter.clear
+
+        # Reload everything.
+        if Facter.respond_to? :loadfacts
+            Facter.loadfacts
+        elsif Facter.respond_to? :load
+            Facter.load
+        else
+            Puppet.warning "You should upgrade your version of Facter to at least 1.3.8"
+        end
+
+        # This loads all existing facts and any new ones.  We have to remove and
+        # reload because there's no way to unload specific facts.
+        loadfacts()
         facts = Facter.to_hash.inject({}) do |newhash, array|
             name, fact = array
             if down
@@ -114,6 +128,9 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
     def getconfig
         dostorage()
 
+        # Retrieve the plugins.
+        getplugins() if Puppet[:pluginsync]
+
         facts = nil
         Puppet::Util.benchmark(:debug, "Retrieved facts") do
             facts = self.class.facts
@@ -121,9 +138,6 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
 
         raise Puppet::Network::ClientError.new("Could not retrieve any facts") unless facts.length > 0
 
-        # Retrieve the plugins.
-        getplugins() if Puppet[:pluginsync]
-
         Puppet.debug("Retrieving catalog")
 
         # If we can't retrieve the catalog, just return, which will either
@@ -339,23 +353,6 @@ class Puppet::Network::Client::Master < Puppet::Network::Client
 
             files << resource[:path]
         end
-    ensure
-        # Clear all existing definitions.
-        Facter.clear
-
-        # Reload everything.
-        if Facter.respond_to? :loadfacts
-            Facter.loadfacts
-        elsif Facter.respond_to? :load
-            Facter.load
-        else
-            raise Puppet::Error,
-                "You must upgrade your version of Facter to use centralized facts"
-        end
-
-        # This loads all existing facts and any new ones.  We have to remove and
-        # reload because there's no way to unload specific facts.
-        loadfacts()
     end
 
     # Retrieve the plugins from the central server.  We only have to load the
diff --git a/lib/puppet/node/catalog.rb b/lib/puppet/node/catalog.rb
index 3ac11a66d..9dbdb8d62 100644
--- a/lib/puppet/node/catalog.rb
+++ b/lib/puppet/node/catalog.rb
@@ -88,8 +88,13 @@ class Puppet::Node::Catalog < Puppet::PGraph
         resource.ref =~ /^(.+)\[/
 
         newref = "%s[%s]" % [$1 || resource.class.name, name]
+
+        # LAK:NOTE It's important that we directly compare the references,
+        # because sometimes an alias is created before the resource is
+        # added to the catalog, so comparing inside the below if block
+        # isn't sufficient.
+        return if newref == resource.ref
         if existing = @resource_table[newref]
-            return if existing == resource
             raise(ArgumentError, "Cannot alias %s to %s; resource %s already exists" % [resource.ref, name, newref])
         end
         @resource_table[newref] = resource
diff --git a/lib/puppet/provider/cron/crontab.rb b/lib/puppet/provider/cron/crontab.rb
index e2228b15e..1cfa0f5fd 100755
--- a/lib/puppet/provider/cron/crontab.rb
+++ b/lib/puppet/provider/cron/crontab.rb
@@ -69,7 +69,11 @@ Puppet::Type.type(:cron).provide(:crontab,
                 end
             end
 
-            str += join(record)
+            if record[:special]
+                str += "@%s %s" % [record[:special], record[:command]]
+            else
+                str += join(record)
+            end
             str
         end
     end
diff --git a/lib/puppet/provider/group/groupadd.rb b/lib/puppet/provider/group/groupadd.rb
index 4df5bf40d..371beee19 100644
--- a/lib/puppet/provider/group/groupadd.rb
+++ b/lib/puppet/provider/group/groupadd.rb
@@ -17,7 +17,7 @@ Puppet::Type.type(:group).provide :groupadd, :parent => Puppet::Provider::NameSe
                 cmd << flag(:gid) << gid
             end
         end
-        if @resource[:allowdupe] == :true
+        if @resource.allowdupe?
             cmd << "-o"
         end
         cmd << @resource[:name]
diff --git a/lib/puppet/provider/nameservice.rb b/lib/puppet/provider/nameservice.rb
index bba6c98ad..9764b5cf8 100644
--- a/lib/puppet/provider/nameservice.rb
+++ b/lib/puppet/provider/nameservice.rb
@@ -323,8 +323,7 @@ class Puppet::Provider::NameService < Puppet::Provider
         begin
             execute(cmd)
         rescue Puppet::ExecutionFailure => detail
-            raise Puppet::Error, "Could not set %s on %s[%s]: %s" %
-                [param, @resource.class.name, @resource.name, detail]
+            raise Puppet::Error, "Could not set %s on %s[%s]: %s" % [param, @resource.class.name, @resource.name, detail]
         end
     end
 end
diff --git a/lib/puppet/provider/nameservice/objectadd.rb b/lib/puppet/provider/nameservice/objectadd.rb
index 4682b5169..b7efe8388 100644
--- a/lib/puppet/provider/nameservice/objectadd.rb
+++ b/lib/puppet/provider/nameservice/objectadd.rb
@@ -22,10 +22,8 @@ class ObjectAdd < Puppet::Provider::NameService
     end
 
     def modifycmd(param, value)
-        cmd = [command(:modify),
-            flag(param),
-            value]
-        if @resource[:allowdupe] == :true && param == :uid
+        cmd = [command(:modify), flag(param), value]
+        if @resource.allowdupe? && ((param == :uid and self.class.name == :useradd) || (param == :gid and self.class.name == :groupadd))
             cmd << "-o"
         end
         cmd << @resource[:name]
@@ -41,4 +39,3 @@ class ObjectAdd < Puppet::Provider::NameService
     end
 end
 end
-
diff --git a/lib/puppet/provider/service/init.rb b/lib/puppet/provider/service/init.rb
index 3081d0eb8..e95fbd0f9 100755
--- a/lib/puppet/provider/service/init.rb
+++ b/lib/puppet/provider/service/init.rb
@@ -13,6 +13,8 @@ Puppet::Type.type(:service).provide :init, :parent => :base do
     case Facter["operatingsystem"].value
     when "FreeBSD":
         @defpath = ["/etc/rc.d", "/usr/local/etc/rc.d"]
+    when "HP-UX":
+        @defpath = "/sbin/init.d"
     else
         @defpath = "/etc/init.d"
     end
diff --git a/lib/puppet/provider/user/useradd.rb b/lib/puppet/provider/user/useradd.rb
index e64601ee0..b327db384 100644
--- a/lib/puppet/provider/user/useradd.rb
+++ b/lib/puppet/provider/user/useradd.rb
@@ -25,7 +25,7 @@ Puppet::Type.type(:user).provide :useradd, :parent => Puppet::Provider::NameServ
 
     def addcmd
         cmd = [command(:add)]
-        @resource.class.validproperties.each do |property|
+        Puppet::Type.type(:user).validproperties.each do |property|
             next if property == :ensure
             # the value needs to be quoted, mostly because -c might
             # have spaces in it
diff --git a/lib/puppet/util/ldap/connection.rb b/lib/puppet/util/ldap/connection.rb
index abcc07ecb..861539872 100644
--- a/lib/puppet/util/ldap/connection.rb
+++ b/lib/puppet/util/ldap/connection.rb
@@ -8,6 +8,18 @@ class Puppet::Util::Ldap::Connection
 
     attr_reader :connection
 
+    # Return a default connection, using our default settings.
+    def self.instance
+        ssl = if Puppet[:ldaptls]
+                  :tls
+              elsif Puppet[:ldapssl]
+                  true
+              else
+                  false
+              end
+        new(Puppet[:ldapserver], Puppet[:ldapport], :ssl => ssl)
+    end
+
     def close
         connection.unbind if connection.bound?
     end
@@ -51,6 +63,7 @@ class Puppet::Util::Ldap::Connection
             @connection.set_option(LDAP::LDAP_OPT_REFERRALS, LDAP::LDAP_OPT_ON)
             @connection.simple_bind(user, password)
         rescue => detail
+            puts detail.class
             raise Puppet::Error, "Could not connect to LDAP: %s" % detail
         end
     end
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index eec86625d..aeda88e09 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -218,6 +218,64 @@ class Puppet::Util::Settings
         @used = []
     end
 
+    # NOTE: ACS ahh the util classes. . .sigh
+    # as part of a fix for 1183, I pulled the logic for the following 5 methods out of the executables and puppet.rb
+    # They probably deserve their own class, but I don't want to do that until I can refactor environments
+    # its a little better than where they were
+
+    # Prints the contents of a config file with the available config elements, or it
+    # prints a single value of a config element.
+    def print_config_options
+        env = value(:environment)
+        val = value(:configprint)
+        if val == "all"
+            hash = {}
+            each do |name, obj|
+                val = value(name,env)
+                val = val.inspect if val == ""
+                hash[name] = val
+            end
+            hash.sort { |a,b| a[0].to_s <=> b[0].to_s }.each do |name, val|
+                puts "%s = %s" % [name, val]
+            end
+        else
+            val.split(/\s*,\s*/).sort.each do |v|
+                if include?(v)
+                    #if there is only one value, just print it for back compatibility
+                    if v == val
+                         puts value(val,env)
+                         break
+                    end
+                    puts "%s = %s" % [v, value(v,env)]
+                else
+                    puts "invalid parameter: %s" % v
+                    return false
+                end
+            end
+        end
+        true
+    end
+
+    def generate_config
+        puts to_config
+        true
+    end
+
+    def generate_manifest
+        puts to_manifest
+        true
+    end
+
+    def print_configs
+        return print_config_options if value(:configprint) != ""
+        return generate_config if value(:genconfig)
+        return generate_manifest if value(:genmanifest)
+    end
+
+    def print_configs?
+        return (value(:configprint) != "" || value(:genconfig) || value(:genmanifest)) && true
+    end
+
     # Return a given object's file metadata.
     def metadata(param)
         if obj = @config[symbolize(param)] and obj.is_a?(CFile)