From 422dea05e755f0026c7786b46a7e93624da1ac0a Mon Sep 17 00:00:00 2001
From: Andrew Shafer <andrew@reductivelabs.com>
Date: Mon, 16 Jun 2008 02:05:38 -0600
Subject: issue 1183

Added environment awareness to --configprint

Pulled the logic for --configprint --genconfig and --genmanifest out of puppet.rb
Put the logic in lib/puppet/util/settings.rb and refactored it a bit

Added specs for the behavior

Reformated the whole spec file to use nested describe

Added the new method to the executables

The old behavior should be preserved, except for the env is now used
---
 bin/puppet                  |    5 +-
 bin/puppetca                |    5 +-
 bin/puppetd                 |    5 +-
 bin/puppetmasterd           |    5 +-
 lib/puppet.rb               |   43 --
 lib/puppet/util/settings.rb |   58 ++
 spec/unit/util/settings.rb  | 1286 +++++++++++++++++++++++--------------------
 7 files changed, 771 insertions(+), 636 deletions(-)

diff --git a/bin/puppet b/bin/puppet
index bb7308997..23b175087 100755
--- a/bin/puppet
+++ b/bin/puppet
@@ -141,8 +141,9 @@ if Puppet[:config] and File.exists? Puppet[:config]
     Puppet.settings.parse(Puppet[:config])
 end
 
-Puppet.genconfig
-Puppet.genmanifest
+if Puppet.settings.print_configs?
+    exit(Puppet.settings.print_configs ? 0 : 1)
+end
 
 # If noop is set, then also enable diffs
 if Puppet[:noop]
diff --git a/bin/puppetca b/bin/puppetca
index 759b602ac..9d88a564e 100755
--- a/bin/puppetca
+++ b/bin/puppetca
@@ -171,8 +171,9 @@ end
 # Now parse the config
 Puppet.parse_config
 
-Puppet.genconfig
-Puppet.genmanifest
+if Puppet.settings.print_configs?
+    exit(Puppet.settings.print_configs ? 0 : 1)
+end
 
 begin
     ca = Puppet::SSLCertificates::CA.new()
diff --git a/bin/puppetd b/bin/puppetd
index 2a71c3a8d..0813745cc 100755
--- a/bin/puppetd
+++ b/bin/puppetd
@@ -299,8 +299,9 @@ unless options[:setdest]
     Puppet::Util::Log.newdestination(:syslog)
 end
 
-Puppet.genconfig
-Puppet.genmanifest
+if Puppet.settings.print_configs?
+    exit(Puppet.settings.print_configs ? 0 : 1)
+end
 
 # If noop is set, then also enable diffs
 if Puppet[:noop]
diff --git a/bin/puppetmasterd b/bin/puppetmasterd
index b4733e604..03bd78455 100755
--- a/bin/puppetmasterd
+++ b/bin/puppetmasterd
@@ -182,8 +182,9 @@ unless options[:setdest]
     Puppet::Util::Log.newdestination(:syslog)
 end
 
-Puppet.genconfig
-Puppet.genmanifest
+if Puppet.settings.print_configs?
+    exit(Puppet.settings.print_configs ? 0 : 1)
+end
 
 # A temporary solution, to at least make the master work for now.
 Puppet::Node::Facts.terminus_class = :yaml
diff --git a/lib/puppet.rb b/lib/puppet.rb
index 66a52f9e3..83e5da68f 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/util/settings.rb b/lib/puppet/util/settings.rb
index 0e6f91e48..d8197f743 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -217,6 +217,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)
diff --git a/spec/unit/util/settings.rb b/spec/unit/util/settings.rb
index a6b358462..de6d6457c 100755
--- a/spec/unit/util/settings.rb
+++ b/spec/unit/util/settings.rb
@@ -2,705 +2,821 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-describe Puppet::Util::Settings, " when specifying defaults" do
-    before do
-        @settings = Puppet::Util::Settings.new
-    end
+describe Puppet::Util::Settings do 
+    describe "when specifying defaults" do
+        before do
+            @settings = Puppet::Util::Settings.new
+        end
 
-    it "should start with no defined parameters" do
-        @settings.params.length.should == 0
-    end
+        it "should start with no defined parameters" do
+            @settings.params.length.should == 0
+        end
 
-    it "should allow specification of default values associated with a section as an array" do
-        @settings.setdefaults(:section, :myvalue => ["defaultval", "my description"])
-    end
+        it "should allow specification of default values associated with a section as an array" do
+            @settings.setdefaults(:section, :myvalue => ["defaultval", "my description"])
+        end
 
-    it "should not allow duplicate parameter specifications" do
-        @settings.setdefaults(:section, :myvalue => ["a", "b"])
-        lambda { @settings.setdefaults(:section, :myvalue => ["c", "d"]) }.should raise_error(ArgumentError)
-    end
+        it "should not allow duplicate parameter specifications" do
+            @settings.setdefaults(:section, :myvalue => ["a", "b"])
+            lambda { @settings.setdefaults(:section, :myvalue => ["c", "d"]) }.should raise_error(ArgumentError)
+        end
 
-    it "should allow specification of default values associated with a section as a hash" do
-        @settings.setdefaults(:section, :myvalue => {:default => "defaultval", :desc => "my description"})
-    end
+        it "should allow specification of default values associated with a section as a hash" do
+            @settings.setdefaults(:section, :myvalue => {:default => "defaultval", :desc => "my description"})
+        end
 
-    it "should consider defined parameters to be valid" do
-        @settings.setdefaults(:section, :myvalue => ["defaultval", "my description"])
-        @settings.valid?(:myvalue).should be_true
-    end
+        it "should consider defined parameters to be valid" do
+            @settings.setdefaults(:section, :myvalue => ["defaultval", "my description"])
+            @settings.valid?(:myvalue).should be_true
+        end
 
-    it "should require a description when defaults are specified with an array" do
-        lambda { @settings.setdefaults(:section, :myvalue => ["a value"]) }.should raise_error(ArgumentError)
-    end
+        it "should require a description when defaults are specified with an array" do
+            lambda { @settings.setdefaults(:section, :myvalue => ["a value"]) }.should raise_error(ArgumentError)
+        end
 
-    it "should require a description when defaults are specified with a hash" do
-        lambda { @settings.setdefaults(:section, :myvalue => {:default => "a value"}) }.should raise_error(ArgumentError)
-    end
+        it "should require a description when defaults are specified with a hash" do
+            lambda { @settings.setdefaults(:section, :myvalue => {:default => "a value"}) }.should raise_error(ArgumentError)
+        end
 
-    it "should support specifying owner, group, and mode when specifying files" do
-        @settings.setdefaults(:section, :myvalue => {:default => "/some/file", :owner => "blah", :mode => "boo", :group => "yay", :desc => "whatever"})
-    end
+        it "should support specifying owner, group, and mode when specifying files" do
+            @settings.setdefaults(:section, :myvalue => {:default => "/some/file", :owner => "blah", :mode => "boo", :group => "yay", :desc => "whatever"})
+        end
 
-    it "should support specifying a short name" do
-        @settings.setdefaults(:section, :myvalue => {:default => "w", :desc => "b", :short => "m"})
-    end
+        it "should support specifying a short name" do
+            @settings.setdefaults(:section, :myvalue => {:default => "w", :desc => "b", :short => "m"})
+        end
 
-    it "should fail when short names conflict" do
-        @settings.setdefaults(:section, :myvalue => {:default => "w", :desc => "b", :short => "m"})
-        lambda { @settings.setdefaults(:section, :myvalue => {:default => "w", :desc => "b", :short => "m"}) }.should raise_error(ArgumentError)
+        it "should fail when short names conflict" do
+            @settings.setdefaults(:section, :myvalue => {:default => "w", :desc => "b", :short => "m"})
+            lambda { @settings.setdefaults(:section, :myvalue => {:default => "w", :desc => "b", :short => "m"}) }.should raise_error(ArgumentError)
+        end
     end
-end
 
-describe Puppet::Util::Settings, " when setting values" do
-    before do
-        @settings = Puppet::Util::Settings.new
-        @settings.setdefaults :main, :myval => ["val", "desc"]
-        @settings.setdefaults :main, :bool => [true, "desc"]
-    end
+    describe "when setting values" do
+        before do
+            @settings = Puppet::Util::Settings.new
+            @settings.setdefaults :main, :myval => ["val", "desc"]
+            @settings.setdefaults :main, :bool => [true, "desc"]
+        end
 
-    it "should provide a method for setting values from other objects" do
-        @settings[:myval] = "something else"
-        @settings[:myval].should == "something else"
-    end
+        it "should provide a method for setting values from other objects" do
+            @settings[:myval] = "something else"
+            @settings[:myval].should == "something else"
+        end
 
-    it "should support a getopt-specific mechanism for setting values" do
-        @settings.handlearg("--myval", "newval")
-        @settings[:myval].should == "newval"
-    end
+        it "should support a getopt-specific mechanism for setting values" do
+            @settings.handlearg("--myval", "newval")
+            @settings[:myval].should == "newval"
+        end
 
-    it "should support a getopt-specific mechanism for turning booleans off" do
-        @settings.handlearg("--no-bool")
-        @settings[:bool].should == false
-    end
+        it "should support a getopt-specific mechanism for turning booleans off" do
+            @settings.handlearg("--no-bool")
+            @settings[:bool].should == false
+        end
 
-    it "should support a getopt-specific mechanism for turning booleans on" do
-        # Turn it off first
-        @settings[:bool] = false
-        @settings.handlearg("--bool")
-        @settings[:bool].should == true
-    end
+        it "should support a getopt-specific mechanism for turning booleans on" do
+            # Turn it off first
+            @settings[:bool] = false
+            @settings.handlearg("--bool")
+            @settings[:bool].should == true
+        end
 
-    it "should clear the cache when setting getopt-specific values" do
-        @settings.setdefaults :mysection, :one => ["whah", "yay"], :two => ["$one yay", "bah"]
-        @settings[:two].should == "whah yay"
-        @settings.handlearg("--one", "else")
-        @settings[:two].should == "else yay"
-    end
+        it "should clear the cache when setting getopt-specific values" do
+            @settings.setdefaults :mysection, :one => ["whah", "yay"], :two => ["$one yay", "bah"]
+            @settings[:two].should == "whah yay"
+            @settings.handlearg("--one", "else")
+            @settings[:two].should == "else yay"
+        end
 
-    it "should not clear other values when setting getopt-specific values" do
-        @settings[:myval] = "yay"
-        @settings.handlearg("--no-bool")
-        @settings[:myval].should == "yay"
-    end
+        it "should not clear other values when setting getopt-specific values" do
+            @settings[:myval] = "yay"
+            @settings.handlearg("--no-bool")
+            @settings[:myval].should == "yay"
+        end
 
-    it "should call passed blocks when values are set" do
-        values = []
-        @settings.setdefaults(:section, :hooker => {:default => "yay", :desc => "boo", :hook => lambda { |v| values << v }})
-        values.should == []
+        it "should call passed blocks when values are set" do
+            values = []
+            @settings.setdefaults(:section, :hooker => {:default => "yay", :desc => "boo", :hook => lambda { |v| values << v }})
+            values.should == []
 
-        @settings[:hooker] = "something"
-        values.should == %w{something}
-    end
+            @settings[:hooker] = "something"
+            values.should == %w{something}
+        end
 
-    it "should provide an option to call passed blocks during definition" do
-        values = []
-        @settings.setdefaults(:section, :hooker => {:default => "yay", :desc => "boo", :call_on_define => true, :hook => lambda { |v| values << v }})
-        values.should == %w{yay}
-    end
+        it "should provide an option to call passed blocks during definition" do
+            values = []
+            @settings.setdefaults(:section, :hooker => {:default => "yay", :desc => "boo", :call_on_define => true, :hook => lambda { |v| values << v }})
+            values.should == %w{yay}
+        end
 
-    it "should pass the fully interpolated value to the hook when called on definition" do
-        values = []
-        @settings.setdefaults(:section, :one => ["test", "a"])
-        @settings.setdefaults(:section, :hooker => {:default => "$one/yay", :desc => "boo", :call_on_define => true, :hook => lambda { |v| values << v }})
-        values.should == %w{test/yay}
-    end
+        it "should pass the fully interpolated value to the hook when called on definition" do
+            values = []
+            @settings.setdefaults(:section, :one => ["test", "a"])
+            @settings.setdefaults(:section, :hooker => {:default => "$one/yay", :desc => "boo", :call_on_define => true, :hook => lambda { |v| values << v }})
+            values.should == %w{test/yay}
+        end
 
-    it "should munge values using the element-specific methods" do
-        @settings[:bool] = "false"
-        @settings[:bool].should == false
-    end
+        it "should munge values using the element-specific methods" do
+            @settings[:bool] = "false"
+            @settings[:bool].should == false
+        end
 
-    it "should prefer cli values to values set in Ruby code" do
-        @settings.handlearg("--myval", "cliarg")
-        @settings[:myval] = "memarg"
-        @settings[:myval].should == "cliarg"
+        it "should prefer cli values to values set in Ruby code" do
+            @settings.handlearg("--myval", "cliarg")
+            @settings[:myval] = "memarg"
+            @settings[:myval].should == "cliarg"
+        end
     end
-end
 
-describe Puppet::Util::Settings, " when returning values" do
-    before do
-        @settings = Puppet::Util::Settings.new
-        @settings.setdefaults :section, :one => ["ONE", "a"], :two => ["$one TWO", "b"], :three => ["$one $two THREE", "c"], :four => ["$two $three FOUR", "d"]
-    end
+    describe "when returning values" do
+        before do
+            @settings = Puppet::Util::Settings.new
+            @settings.setdefaults :section, :one => ["ONE", "a"], :two => ["$one TWO", "b"], :three => ["$one $two THREE", "c"], :four => ["$two $three FOUR", "d"]
+        end
 
-    it "should provide a mechanism for returning set values" do
-        @settings[:one] = "other"
-        @settings[:one].should == "other"
-    end
+        it "should provide a mechanism for returning set values" do
+            @settings[:one] = "other"
+            @settings[:one].should == "other"
+        end
 
-    it "should interpolate default values for other parameters into returned parameter values" do
-        @settings[:one].should == "ONE"
-        @settings[:two].should == "ONE TWO"
-        @settings[:three].should == "ONE ONE TWO THREE"
-    end
+        it "should interpolate default values for other parameters into returned parameter values" do
+            @settings[:one].should == "ONE"
+            @settings[:two].should == "ONE TWO"
+            @settings[:three].should == "ONE ONE TWO THREE"
+        end
 
-    it "should interpolate default values that themselves need to be interpolated" do
-        @settings[:four].should == "ONE TWO ONE ONE TWO THREE FOUR"
-    end
+        it "should interpolate default values that themselves need to be interpolated" do
+            @settings[:four].should == "ONE TWO ONE ONE TWO THREE FOUR"
+        end
 
-    it "should interpolate set values for other parameters into returned parameter values" do
-        @settings[:one] = "on3"
-        @settings[:two] = "$one tw0"
-        @settings[:three] = "$one $two thr33"
-        @settings[:four] = "$one $two $three f0ur"
-        @settings[:one].should == "on3"
-        @settings[:two].should == "on3 tw0"
-        @settings[:three].should == "on3 on3 tw0 thr33"
-        @settings[:four].should == "on3 on3 tw0 on3 on3 tw0 thr33 f0ur"
-    end
+        it "should interpolate set values for other parameters into returned parameter values" do
+            @settings[:one] = "on3"
+            @settings[:two] = "$one tw0"
+            @settings[:three] = "$one $two thr33"
+            @settings[:four] = "$one $two $three f0ur"
+            @settings[:one].should == "on3"
+            @settings[:two].should == "on3 tw0"
+            @settings[:three].should == "on3 on3 tw0 thr33"
+            @settings[:four].should == "on3 on3 tw0 on3 on3 tw0 thr33 f0ur"
+        end
 
-    it "should not cache interpolated values such that stale information is returned" do
-        @settings[:two].should == "ONE TWO"
-        @settings[:one] = "one"
-        @settings[:two].should == "one TWO"
-    end
+        it "should not cache interpolated values such that stale information is returned" do
+            @settings[:two].should == "ONE TWO"
+            @settings[:one] = "one"
+            @settings[:two].should == "one TWO"
+        end
 
-    it "should not cache values such that information from one environment is returned for another environment" do
-        text = "[env1]\none = oneval\n[env2]\none = twoval\n"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/whatever")
-        @settings.stubs(:read_file).with(file).returns(text)
-        @settings.parse(file)
+        it "should not cache values such that information from one environment is returned for another environment" do
+            text = "[env1]\none = oneval\n[env2]\none = twoval\n"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/whatever")
+            @settings.stubs(:read_file).with(file).returns(text)
+            @settings.parse(file)
 
-        @settings.value(:one, "env1").should == "oneval"
-        @settings.value(:one, "env2").should == "twoval"
-    end
+            @settings.value(:one, "env1").should == "oneval"
+            @settings.value(:one, "env2").should == "twoval"
+        end
 
-    it "should have a name determined by the 'name' parameter" do
-        @settings.setdefaults(:whatever, :name => ["something", "yayness"])
-        @settings.name.should == :something
-        @settings[:name] = :other
-        @settings.name.should == :other
+        it "should have a name determined by the 'name' parameter" do
+            @settings.setdefaults(:whatever, :name => ["something", "yayness"])
+            @settings.name.should == :something
+            @settings[:name] = :other
+            @settings.name.should == :other
+        end
     end
-end
 
-describe Puppet::Util::Settings, " when choosing which value to return" do
-    before do
-        @settings = Puppet::Util::Settings.new
-        @settings.setdefaults :section,
-            :one => ["ONE", "a"],
-            :name => ["myname", "w"]
-    end
+    describe "when choosing which value to return" do
+        before do
+            @settings = Puppet::Util::Settings.new
+            @settings.setdefaults :section,
+                :one => ["ONE", "a"],
+                :name => ["myname", "w"]
+        end
 
-    it "should return default values if no values have been set" do
-        @settings[:one].should == "ONE"
-    end
+        it "should return default values if no values have been set" do
+            @settings[:one].should == "ONE"
+        end
 
-    it "should return values set on the cli before values set in the configuration file" do
-        text = "[main]\none = fileval\n"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/whatever")
-        @settings.stubs(:parse_file).returns(text)
-        @settings.handlearg("--one", "clival")
-        @settings.parse(file)
+        it "should return values set on the cli before values set in the configuration file" do
+            text = "[main]\none = fileval\n"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/whatever")
+            @settings.stubs(:parse_file).returns(text)
+            @settings.handlearg("--one", "clival")
+            @settings.parse(file)
 
-        @settings[:one].should == "clival"
-    end
+            @settings[:one].should == "clival"
+        end
 
-    it "should return values set on the cli before values set in Ruby" do
-        @settings[:one] = "rubyval"
-        @settings.handlearg("--one", "clival")
-        @settings[:one].should == "clival"
-    end
+        it "should return values set on the cli before values set in Ruby" do
+            @settings[:one] = "rubyval"
+            @settings.handlearg("--one", "clival")
+            @settings[:one].should == "clival"
+        end
 
-    it "should return values set in the executable-specific section before values set in the main section" do
-        text = "[main]\none = mainval\n[myname]\none = nameval\n"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/whatever")
-        @settings.stubs(:read_file).with(file).returns(text)
-        @settings.parse(file)
+        it "should return values set in the executable-specific section before values set in the main section" do
+            text = "[main]\none = mainval\n[myname]\none = nameval\n"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/whatever")
+            @settings.stubs(:read_file).with(file).returns(text)
+            @settings.parse(file)
 
-        @settings[:one].should == "nameval"
-    end
+            @settings[:one].should == "nameval"
+        end
 
-    it "should not return values outside of its search path" do
-        text = "[other]\none = oval\n"
-        file = "/some/file"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/whatever")
-        @settings.stubs(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings[:one].should == "ONE"
-    end
+        it "should not return values outside of its search path" do
+            text = "[other]\none = oval\n"
+            file = "/some/file"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/whatever")
+            @settings.stubs(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings[:one].should == "ONE"
+        end
 
-    it "should return values in a specified environment" do
-        text = "[env]\none = envval\n"
-        file = "/some/file"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/whatever")
-        @settings.stubs(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings.value(:one, "env").should == "envval"
-    end
+        it "should return values in a specified environment" do
+            text = "[env]\none = envval\n"
+            file = "/some/file"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/whatever")
+            @settings.stubs(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings.value(:one, "env").should == "envval"
+        end
 
-    it "should return values in a specified environment before values in the main or name sections" do
-        text = "[env]\none = envval\n[main]\none = mainval\n[myname]\none = nameval\n"
-        file = "/some/file"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/whatever")
-        @settings.stubs(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings.value(:one, "env").should == "envval"
+        it "should return values in a specified environment before values in the main or name sections" do
+            text = "[env]\none = envval\n[main]\none = mainval\n[myname]\none = nameval\n"
+            file = "/some/file"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/whatever")
+            @settings.stubs(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings.value(:one, "env").should == "envval"
+        end
     end
-end
 
-describe Puppet::Util::Settings, " when parsing its configuration" do
-    before do
-        @settings = Puppet::Util::Settings.new
-        @settings.setdefaults :section, :one => ["ONE", "a"], :two => ["$one TWO", "b"], :three => ["$one $two THREE", "c"]
-    end
+    describe "when parsing its configuration" do
+        before do
+            @settings = Puppet::Util::Settings.new
+            @settings.setdefaults :section, :one => ["ONE", "a"], :two => ["$one TWO", "b"], :three => ["$one $two THREE", "c"]
+        end
 
-    it "should return values set in the configuration file" do
-        text = "[main]
-        one = fileval
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings[:one].should == "fileval"
-    end
+        it "should return values set in the configuration file" do
+            text = "[main]
+            one = fileval
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings[:one].should == "fileval"
+        end
 
-    #484 - this should probably be in the regression area
-    it "should not throw an exception on unknown parameters" do
-        text = "[main]\nnosuchparam = mval\n"
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        lambda { @settings.parse(file) }.should_not raise_error
-    end
+        #484 - this should probably be in the regression area
+        it "should not throw an exception on unknown parameters" do
+            text = "[main]\nnosuchparam = mval\n"
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            lambda { @settings.parse(file) }.should_not raise_error
+        end
 
-    it "should support an old parse method when per-executable configuration files still exist" do
-        # I'm not going to bother testing this method.
-        @settings.should respond_to(:old_parse)
-    end
+        it "should support an old parse method when per-executable configuration files still exist" do
+            # I'm not going to bother testing this method.
+            @settings.should respond_to(:old_parse)
+        end
 
-    it "should convert booleans in the configuration file into Ruby booleans" do
-        text = "[main]
-        one = true
-        two = false
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings[:one].should == true
-        @settings[:two].should == false
-    end
+        it "should convert booleans in the configuration file into Ruby booleans" do
+            text = "[main]
+            one = true
+            two = false
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings[:one].should == true
+            @settings[:two].should == false
+        end
 
-    it "should convert integers in the configuration file into Ruby Integers" do
-        text = "[main]
-        one = 65
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings[:one].should == 65
-    end
+        it "should convert integers in the configuration file into Ruby Integers" do
+            text = "[main]
+            one = 65
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings[:one].should == 65
+        end
 
-    it "should support specifying all metadata (owner, group, mode) in the configuration file" do
-        @settings.setdefaults :section, :myfile => ["/myfile", "a"]
-
-        text = "[main]
-        myfile = /other/file {owner = luke, group = luke, mode = 644}
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings[:myfile].should == "/other/file"
-        @settings.metadata(:myfile).should == {:owner => "luke", :group => "luke", :mode => "644"}
-    end
+        it "should support specifying all metadata (owner, group, mode) in the configuration file" do
+            @settings.setdefaults :section, :myfile => ["/myfile", "a"]
+
+            text = "[main]
+            myfile = /other/file {owner = luke, group = luke, mode = 644}
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings[:myfile].should == "/other/file"
+            @settings.metadata(:myfile).should == {:owner => "luke", :group => "luke", :mode => "644"}
+        end
 
-    it "should support specifying a single piece of metadata (owner, group, or mode) in the configuration file" do
-        @settings.setdefaults :section, :myfile => ["/myfile", "a"]
-
-        text = "[main]
-        myfile = /other/file {owner = luke}
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings[:myfile].should == "/other/file"
-        @settings.metadata(:myfile).should == {:owner => "luke"}
-    end
+        it "should support specifying a single piece of metadata (owner, group, or mode) in the configuration file" do
+            @settings.setdefaults :section, :myfile => ["/myfile", "a"]
+
+            text = "[main]
+            myfile = /other/file {owner = luke}
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings[:myfile].should == "/other/file"
+            @settings.metadata(:myfile).should == {:owner => "luke"}
+        end
 
-    it "should call hooks associated with values set in the configuration file" do
-        values = []
-        @settings.setdefaults :section, :mysetting => {:default => "defval", :desc => "a", :hook => proc { |v| values << v }}
-
-        text = "[main]
-        mysetting = setval
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        values.should == ["setval"]
-    end
+        it "should call hooks associated with values set in the configuration file" do
+            values = []
+            @settings.setdefaults :section, :mysetting => {:default => "defval", :desc => "a", :hook => proc { |v| values << v }}
+
+            text = "[main]
+            mysetting = setval
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            values.should == ["setval"]
+        end
 
-    it "should not call the same hook for values set multiple times in the configuration file" do
-        values = []
-        @settings.setdefaults :section, :mysetting => {:default => "defval", :desc => "a", :hook => proc { |v| values << v }}
-
-        text = "[main]
-        mysetting = setval
-        [puppet]
-        mysetting = other
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        values.should == ["setval"]
-    end
+        it "should not call the same hook for values set multiple times in the configuration file" do
+            values = []
+            @settings.setdefaults :section, :mysetting => {:default => "defval", :desc => "a", :hook => proc { |v| values << v }}
+
+            text = "[main]
+            mysetting = setval
+            [puppet]
+            mysetting = other
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            values.should == ["setval"]
+        end
 
-    it "should pass the environment-specific value to the hook when one is available" do
-        values = []
-        @settings.setdefaults :section, :mysetting => {:default => "defval", :desc => "a", :hook => proc { |v| values << v }}
-        @settings.setdefaults :section, :environment => ["yay", "a"]
-        @settings.setdefaults :section, :environments => ["yay,foo", "a"]
-
-        text = "[main]
-        mysetting = setval
-        [yay]
-        mysetting = other
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        values.should == ["other"]
-    end
+        it "should pass the environment-specific value to the hook when one is available" do
+            values = []
+            @settings.setdefaults :section, :mysetting => {:default => "defval", :desc => "a", :hook => proc { |v| values << v }}
+            @settings.setdefaults :section, :environment => ["yay", "a"]
+            @settings.setdefaults :section, :environments => ["yay,foo", "a"]
+
+            text = "[main]
+            mysetting = setval
+            [yay]
+            mysetting = other
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            values.should == ["other"]
+        end
 
-    it "should pass the interpolated value to the hook when one is available" do
-        values = []
-        @settings.setdefaults :section, :base => {:default => "yay", :desc => "a", :hook => proc { |v| values << v }}
-        @settings.setdefaults :section, :mysetting => {:default => "defval", :desc => "a", :hook => proc { |v| values << v }}
-
-        text = "[main]
-        mysetting = $base/setval
-        "
-        file = "/some/file"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        values.should == ["yay/setval"]
-    end
+        it "should pass the interpolated value to the hook when one is available" do
+            values = []
+            @settings.setdefaults :section, :base => {:default => "yay", :desc => "a", :hook => proc { |v| values << v }}
+            @settings.setdefaults :section, :mysetting => {:default => "defval", :desc => "a", :hook => proc { |v| values << v }}
+
+            text = "[main]
+            mysetting = $base/setval
+            "
+            file = "/some/file"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            values.should == ["yay/setval"]
+        end
 
-    it "should allow empty values" do
-        @settings.setdefaults :section, :myarg => ["myfile", "a"]
+        it "should allow empty values" do
+            @settings.setdefaults :section, :myarg => ["myfile", "a"]
 
-        text = "[main]
-        myarg =
-        "
-        @settings.stubs(:read_file).returns(text)
-        @settings.parse("/some/file")
-        @settings[:myarg].should == ""
+            text = "[main]
+            myarg =
+            "
+            @settings.stubs(:read_file).returns(text)
+            @settings.parse("/some/file")
+            @settings[:myarg].should == ""
+        end
     end
-end
 
-describe Puppet::Util::Settings, " when reparsing its configuration" do
-    before do
-        @settings = Puppet::Util::Settings.new
-        @settings.setdefaults :section, :one => ["ONE", "a"], :two => ["$one TWO", "b"], :three => ["$one $two THREE", "c"]
-    end
+    describe "when reparsing its configuration" do
+        before do
+            @settings = Puppet::Util::Settings.new
+            @settings.setdefaults :section, :one => ["ONE", "a"], :two => ["$one TWO", "b"], :three => ["$one $two THREE", "c"]
+        end
 
-    it "should replace in-memory values with on-file values" do
-        # Init the value
-        text = "[main]\none = disk-init\n"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/test/file")
-        @settings[:one] = "init"
-        @settings.file = file
-
-        # Now replace the value
-        text = "[main]\none = disk-replace\n"
-        
-        # This is kinda ridiculous - the reason it parses twice is that
-        # it goes to parse again when we ask for the value, because the
-        # mock always says it should get reparsed.
-        @settings.expects(:read_file).with(file).returns(text).times(2)
-        @settings.reparse
-        @settings[:one].should == "disk-replace"
-    end
+        it "should replace in-memory values with on-file values" do
+            # Init the value
+            text = "[main]\none = disk-init\n"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/test/file")
+            @settings[:one] = "init"
+            @settings.file = file
+
+            # Now replace the value
+            text = "[main]\none = disk-replace\n"
+            
+            # This is kinda ridiculous - the reason it parses twice is that
+            # it goes to parse again when we ask for the value, because the
+            # mock always says it should get reparsed.
+            @settings.expects(:read_file).with(file).returns(text).times(2)
+            @settings.reparse
+            @settings[:one].should == "disk-replace"
+        end
 
-    it "should retain parameters set by cli when configuration files are reparsed" do
-        @settings.handlearg("--one", "clival")
+        it "should retain parameters set by cli when configuration files are reparsed" do
+            @settings.handlearg("--one", "clival")
 
-        text = "[main]\none = on-disk\n"
-        file = mock 'file'
-        file.stubs(:file).returns("/test/file")
-        @settings.stubs(:read_file).with(file).returns(text)
-        @settings.parse(file)
+            text = "[main]\none = on-disk\n"
+            file = mock 'file'
+            file.stubs(:file).returns("/test/file")
+            @settings.stubs(:read_file).with(file).returns(text)
+            @settings.parse(file)
 
-        @settings[:one].should == "clival"
-    end
+            @settings[:one].should == "clival"
+        end
 
-    it "should remove in-memory values that are no longer set in the file" do
-        # Init the value
-        text = "[main]\none = disk-init\n"
-        file = mock 'file'
-        file.stubs(:changed?).returns(true)
-        file.stubs(:file).returns("/test/file")
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        @settings[:one].should == "disk-init"
-
-        # Now replace the value
-        text = "[main]\ntwo = disk-replace\n"
-        @settings.expects(:read_file).with(file).returns(text)
-        @settings.parse(file)
-        #@settings.reparse
-
-        # The originally-overridden value should be replaced with the default
-        @settings[:one].should == "ONE"
-
-        # and we should now have the new value in memory
-        @settings[:two].should == "disk-replace"
+        it "should remove in-memory values that are no longer set in the file" do
+            # Init the value
+            text = "[main]\none = disk-init\n"
+            file = mock 'file'
+            file.stubs(:changed?).returns(true)
+            file.stubs(:file).returns("/test/file")
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            @settings[:one].should == "disk-init"
+
+            # Now replace the value
+            text = "[main]\ntwo = disk-replace\n"
+            @settings.expects(:read_file).with(file).returns(text)
+            @settings.parse(file)
+            #@settings.reparse
+
+            # The originally-overridden value should be replaced with the default
+            @settings[:one].should == "ONE"
+
+            # and we should now have the new value in memory
+            @settings[:two].should == "disk-replace"
+        end
     end
-end
 
-describe Puppet::Util::Settings, " when being used to manage the host machine" do
-    before do
-        @settings = Puppet::Util::Settings.new
-        @settings.setdefaults :main, :maindir => ["/maindir", "a"], :seconddir => ["/seconddir", "a"]
-        @settings.setdefaults :other, :otherdir => {:default => "/otherdir", :desc => "a", :owner => "luke", :group => "johnny", :mode => 0755}
-        @settings.setdefaults :third, :thirddir => ["/thirddir", "b"]
-        @settings.setdefaults :files, :myfile => {:default => "/myfile", :desc => "a", :mode => 0755}
-    end
+    describe "when being used to manage the host machine" do
+        before do
+            @settings = Puppet::Util::Settings.new
+            @settings.setdefaults :main, :maindir => ["/maindir", "a"], :seconddir => ["/seconddir", "a"]
+            @settings.setdefaults :other, :otherdir => {:default => "/otherdir", :desc => "a", :owner => "luke", :group => "johnny", :mode => 0755}
+            @settings.setdefaults :third, :thirddir => ["/thirddir", "b"]
+            @settings.setdefaults :files, :myfile => {:default => "/myfile", :desc => "a", :mode => 0755}
+        end
 
-    def stub_transaction
-        @bucket = mock 'bucket'
-        @config = mock 'config', :clear => nil
-        @trans = mock 'transaction'
+        def stub_transaction
+            @bucket = mock 'bucket'
+            @config = mock 'config', :clear => nil
+            @trans = mock 'transaction'
 
-        @settings.expects(:to_transportable).with(:whatever).returns(@bucket)
-        @bucket.expects(:to_catalog).returns(@config)
-        @config.expects(:apply).yields(@trans)
-        @config.stubs(:host_config=)
-    end
+            @settings.expects(:to_transportable).with(:whatever).returns(@bucket)
+            @bucket.expects(:to_catalog).returns(@config)
+            @config.expects(:apply).yields(@trans)
+            @config.stubs(:host_config=)
+        end
 
-    it "should provide a method that writes files with the correct modes" do
-        pending "Not converted from test/unit yet"
-    end
+        it "should provide a method that writes files with the correct modes" do
+            pending "Not converted from test/unit yet"
+        end
 
-    it "should provide a method that creates directories with the correct modes" do
-        Puppet::Util::SUIDManager.expects(:asuser).with("luke", "johnny").yields
-        Dir.expects(:mkdir).with("/otherdir", 0755)
-        @settings.mkdir(:otherdir)
-    end
+        it "should provide a method that creates directories with the correct modes" do
+            Puppet::Util::SUIDManager.expects(:asuser).with("luke", "johnny").yields
+            Dir.expects(:mkdir).with("/otherdir", 0755)
+            @settings.mkdir(:otherdir)
+        end
 
-    it "should be able to create needed directories in a single section" do
-        Dir.expects(:mkdir).with("/maindir")
-        Dir.expects(:mkdir).with("/seconddir")
-        @settings.use(:main)
-    end
+        it "should be able to create needed directories in a single section" do
+            Dir.expects(:mkdir).with("/maindir")
+            Dir.expects(:mkdir).with("/seconddir")
+            @settings.use(:main)
+        end
 
-    it "should be able to create needed directories in multiple sections" do
-        Dir.expects(:mkdir).with("/maindir")
-        Dir.expects(:mkdir).with("/seconddir")
-        Dir.expects(:mkdir).with("/thirddir")
-        @settings.use(:main, :third)
-    end
+        it "should be able to create needed directories in multiple sections" do
+            Dir.expects(:mkdir).with("/maindir")
+            Dir.expects(:mkdir).with("/seconddir")
+            Dir.expects(:mkdir).with("/thirddir")
+            @settings.use(:main, :third)
+        end
 
-    it "should provide a method to trigger enforcing of file modes on existing files and directories" do
-        pending "Not converted from test/unit yet"
-    end
+        it "should provide a method to trigger enforcing of file modes on existing files and directories" do
+            pending "Not converted from test/unit yet"
+        end
 
-    it "should provide a method to convert the file mode enforcement into a Puppet manifest" do
-        pending "Not converted from test/unit yet"
-    end
+        it "should provide a method to convert the file mode enforcement into a Puppet manifest" do
+            pending "Not converted from test/unit yet"
+        end
 
-    it "should create files when configured to do so with the :create parameter"
-
-    it "should provide a method to convert the file mode enforcement into transportable resources" do
-        # Make it think we're root so it tries to manage user and group.
-        Puppet.features.stubs(:root?).returns(true)
-        File.stubs(:exist?).with("/myfile").returns(true)
-        trans = nil
-        trans = @settings.to_transportable
-        resources = []
-        trans.delve { |obj| resources << obj if obj.is_a? Puppet::TransObject }
-        %w{/maindir /seconddir /otherdir /myfile}.each do |path|
-            obj = resources.find { |r| r.type == "file" and r.name == path }
-            if path.include?("dir")
-                obj[:ensure].should == :directory
-            else
-                # Do not create the file, just manage mode
-                obj[:ensure].should be_nil
+        it "should create files when configured to do so with the :create parameter"
+
+        it "should provide a method to convert the file mode enforcement into transportable resources" do
+            # Make it think we're root so it tries to manage user and group.
+            Puppet.features.stubs(:root?).returns(true)
+            File.stubs(:exist?).with("/myfile").returns(true)
+            trans = nil
+            trans = @settings.to_transportable
+            resources = []
+            trans.delve { |obj| resources << obj if obj.is_a? Puppet::TransObject }
+            %w{/maindir /seconddir /otherdir /myfile}.each do |path|
+                obj = resources.find { |r| r.type == "file" and r.name == path }
+                if path.include?("dir")
+                    obj[:ensure].should == :directory
+                else
+                    # Do not create the file, just manage mode
+                    obj[:ensure].should be_nil
+                end
+                obj.should be_instance_of(Puppet::TransObject)
+                case path
+                when "/otherdir":
+                    obj[:owner].should == "luke"
+                    obj[:group].should == "johnny"
+                    obj[:mode].should == 0755
+                when "/myfile":
+                    obj[:mode].should == 0755
+                end
             end
-            obj.should be_instance_of(Puppet::TransObject)
-            case path
-            when "/otherdir":
-                obj[:owner].should == "luke"
-                obj[:group].should == "johnny"
-                obj[:mode].should == 0755
-            when "/myfile":
-                obj[:mode].should == 0755
+        end
+
+        it "should not try to manage user or group when not running as root" do
+            Puppet.features.stubs(:root?).returns(false)
+            trans = nil
+            trans = @settings.to_transportable(:other)
+            trans.delve do |obj|
+                next unless obj.is_a?(Puppet::TransObject)
+                obj[:owner].should be_nil
+                obj[:group].should be_nil
             end
         end
-    end
 
-    it "should not try to manage user or group when not running as root" do
-        Puppet.features.stubs(:root?).returns(false)
-        trans = nil
-        trans = @settings.to_transportable(:other)
-        trans.delve do |obj|
-            next unless obj.is_a?(Puppet::TransObject)
-            obj[:owner].should be_nil
-            obj[:group].should be_nil
+        it "should add needed users and groups to the manifest when asked" do
+            # This is how we enable user/group management
+            @settings.setdefaults :main, :mkusers => [true, "w"]
+            Puppet.features.stubs(:root?).returns(false)
+            trans = nil
+            trans = @settings.to_transportable(:other)
+            resources = []
+            trans.delve { |obj| resources << obj if obj.is_a? Puppet::TransObject and obj.type != "file" }
+
+            user = resources.find { |r| r.type == "user" }
+            user.should be_instance_of(Puppet::TransObject)
+            user.name.should == "luke"
+            user[:ensure].should == :present
+
+            # This should maybe be a separate test, but...
+            group = resources.find { |r| r.type == "group" }
+            group.should be_instance_of(Puppet::TransObject)
+            group.name.should == "johnny"
+            group[:ensure].should == :present
         end
-    end
 
-    it "should add needed users and groups to the manifest when asked" do
-        # This is how we enable user/group management
-        @settings.setdefaults :main, :mkusers => [true, "w"]
-        Puppet.features.stubs(:root?).returns(false)
-        trans = nil
-        trans = @settings.to_transportable(:other)
-        resources = []
-        trans.delve { |obj| resources << obj if obj.is_a? Puppet::TransObject and obj.type != "file" }
-
-        user = resources.find { |r| r.type == "user" }
-        user.should be_instance_of(Puppet::TransObject)
-        user.name.should == "luke"
-        user[:ensure].should == :present
-
-        # This should maybe be a separate test, but...
-        group = resources.find { |r| r.type == "group" }
-        group.should be_instance_of(Puppet::TransObject)
-        group.name.should == "johnny"
-        group[:ensure].should == :present
-    end
+        it "should ignore tags and schedules when creating files and directories"
 
-    it "should ignore tags and schedules when creating files and directories"
+        it "should apply all resources in debug mode to reduce logging"
 
-    it "should apply all resources in debug mode to reduce logging"
+        it "should not try to manage absent files" do
+            # Make it think we're root so it tries to manage user and group.
+            Puppet.features.stubs(:root?).returns(true)
+            trans = nil
+            trans = @settings.to_transportable
+            file = nil
+            trans.delve { |obj| file = obj if obj.name == "/myfile" }
+            file.should be_nil
+        end
 
-    it "should not try to manage absent files" do
-        # Make it think we're root so it tries to manage user and group.
-        Puppet.features.stubs(:root?).returns(true)
-        trans = nil
-        trans = @settings.to_transportable
-        file = nil
-        trans.delve { |obj| file = obj if obj.name == "/myfile" }
-        file.should be_nil
-    end
+        it "should not try to manage files in memory" do
+            main = Puppet::Type.type(:file).create(:path => "/maindir")
 
-    it "should not try to manage files in memory" do
-        main = Puppet::Type.type(:file).create(:path => "/maindir")
+            trans = @settings.to_transportable
 
-        trans = @settings.to_transportable
+            lambda { trans.to_catalog }.should_not raise_error
+        end
 
-        lambda { trans.to_catalog }.should_not raise_error
-    end
+        it "should do nothing if a catalog cannot be created" do
+            bucket = mock 'bucket'
+            catalog = mock 'catalog'
 
-    it "should do nothing if a catalog cannot be created" do
-        bucket = mock 'bucket'
-        catalog = mock 'catalog'
+            @settings.expects(:to_transportable).returns bucket
+            bucket.expects(:to_catalog).raises RuntimeError
+            catalog.expects(:apply).never
 
-        @settings.expects(:to_transportable).returns bucket
-        bucket.expects(:to_catalog).raises RuntimeError
-        catalog.expects(:apply).never
+            @settings.use(:mysection)
+        end
 
-        @settings.use(:mysection)
-    end
+        it "should clear the catalog after applying" do
+            bucket = mock 'bucket'
+            catalog = mock 'catalog'
 
-    it "should clear the catalog after applying" do
-        bucket = mock 'bucket'
-        catalog = mock 'catalog'
+            @settings.expects(:to_transportable).returns bucket
+            bucket.expects(:to_catalog).returns catalog
+            catalog.stubs(:host_config=)
+            catalog.stubs(:apply)
+            catalog.expects(:clear)
 
-        @settings.expects(:to_transportable).returns bucket
-        bucket.expects(:to_catalog).returns catalog
-        catalog.stubs(:host_config=)
-        catalog.stubs(:apply)
-        catalog.expects(:clear)
+            @settings.use(:mysection)
+        end
 
-        @settings.use(:mysection)
-    end
+        it "should clear the catalog even if there is an exception during applying" do
+            bucket = mock 'bucket'
+            catalog = mock 'catalog'
 
-    it "should clear the catalog even if there is an exception during applying" do
-        bucket = mock 'bucket'
-        catalog = mock 'catalog'
+            @settings.expects(:to_transportable).returns bucket
+            bucket.expects(:to_catalog).returns catalog
+            catalog.stubs(:host_config=)
+            catalog.expects(:apply).raises(ArgumentError)
+            catalog.expects(:clear)
 
-        @settings.expects(:to_transportable).returns bucket
-        bucket.expects(:to_catalog).returns catalog
-        catalog.stubs(:host_config=)
-        catalog.expects(:apply).raises(ArgumentError)
-        catalog.expects(:clear)
+            # We don't care about the raised exception, we just care that
+            # we clear the catalog even with the exception
+            lambda { @settings.use(:mysection) }.should raise_error
+        end
 
-        # We don't care about the raised exception, we just care that
-        # we clear the catalog even with the exception
-        lambda { @settings.use(:mysection) }.should raise_error
-    end
+        it "should do nothing if all specified sections have already been used" do
+            bucket = mock 'bucket'
+            catalog = mock 'catalog'
 
-    it "should do nothing if all specified sections have already been used" do
-        bucket = mock 'bucket'
-        catalog = mock 'catalog'
+            @settings.expects(:to_transportable).once.returns(bucket)
+            bucket.expects(:to_catalog).returns catalog
+            catalog.stub_everything
 
-        @settings.expects(:to_transportable).once.returns(bucket)
-        bucket.expects(:to_catalog).returns catalog
-        catalog.stub_everything
+            @settings.use(:whatever)
 
-        @settings.use(:whatever)
+            @settings.use(:whatever)
+        end
 
-        @settings.use(:whatever)
-    end
+        it "should ignore file settings whose values are not strings" do
+            @settings[:maindir] = false
 
-    it "should ignore file settings whose values are not strings" do
-        @settings[:maindir] = false
+            lambda { trans = @settings.to_transportable }.should_not raise_error
+        end
 
-        lambda { trans = @settings.to_transportable }.should_not raise_error
-    end
+        it "should be able to turn the current configuration into a parseable manifest"
 
-    it "should be able to turn the current configuration into a parseable manifest"
+        it "should convert octal numbers correctly when producing a manifest"
 
-    it "should convert octal numbers correctly when producing a manifest"
+        it "should be able to provide all of its parameters in a format compatible with GetOpt::Long" do
+            pending "Not converted from test/unit yet"
+        end
 
-    it "should be able to provide all of its parameters in a format compatible with GetOpt::Long" do
-        pending "Not converted from test/unit yet"
-    end
+        it "should not attempt to manage files within /dev" do
+            pending "Not converted from test/unit yet"
+        end
 
-    it "should not attempt to manage files within /dev" do
-        pending "Not converted from test/unit yet"
-    end
+        it "should not modify the stored state database when managing resources" do
+            Puppet::Util::Storage.expects(:store).never
+            Puppet::Util::Storage.expects(:load).never
+            Dir.expects(:mkdir).with("/maindir")
+            Dir.expects(:mkdir).with("/seconddir")
+            @settings.use(:main)
+        end
 
-    it "should not modify the stored state database when managing resources" do
-        Puppet::Util::Storage.expects(:store).never
-        Puppet::Util::Storage.expects(:load).never
-        Dir.expects(:mkdir).with("/maindir")
-        Dir.expects(:mkdir).with("/seconddir")
-        @settings.use(:main)
-    end
+        it "should convert all relative paths to fully-qualified paths (#795)" do
+            @settings[:myfile] = "unqualified"
+            dir = Dir.getwd
+            @settings[:myfile].should == File.join(dir, "unqualified")
+        end
 
-    it "should convert all relative paths to fully-qualified paths (#795)" do
-        @settings[:myfile] = "unqualified"
-        dir = Dir.getwd
-        @settings[:myfile].should == File.join(dir, "unqualified")
-    end
+        it "should support a method for re-using all currently used sections" do
+            Dir.expects(:mkdir).with("/thirddir").times(2)
+            @settings.use(:third)
+            @settings.reuse
+        end
 
-    it "should support a method for re-using all currently used sections" do
-        Dir.expects(:mkdir).with("/thirddir").times(2)
-        @settings.use(:third)
-        @settings.reuse
-    end
+        it "should fail if any resources fail" do
+            stub_transaction
+            @trans.expects(:any_failed?).returns(true)
 
-    it "should fail if any resources fail" do
-        stub_transaction
-        @trans.expects(:any_failed?).returns(true)
+            proc { @settings.use(:whatever) }.should raise_error(RuntimeError)
+        end
 
-        proc { @settings.use(:whatever) }.should raise_error(RuntimeError)
+        after { Puppet::Type.allclear }
     end
 
-    after { Puppet::Type.allclear }
+    describe "when dealing with printing configs" do
+        before do
+            @settings = Puppet::Util::Settings.new
+            #these are the magic default values
+            @settings.stubs(:value).with(:configprint).returns("")
+            @settings.stubs(:value).with(:genconfig).returns(false)
+            @settings.stubs(:value).with(:genmanifest).returns(false)
+            @settings.stubs(:value).with(:environment).returns(nil)
+        end
+
+        describe "when checking print_config?" do
+            it "should return false when the :configprint, :genconfig and :genmanifest are not set" do
+                @settings.print_configs?.should be_false
+            end
+
+            it "should return true when :configprint has a value" do
+                @settings.stubs(:value).with(:configprint).returns("something")
+                @settings.print_configs?.should be_true
+            end
+
+            it "should return true when :genconfig has a value" do
+                @settings.stubs(:value).with(:genconfig).returns(true)
+                @settings.print_configs?.should be_true
+            end
+
+            it "should return true when :genmanifest has a value" do
+                @settings.stubs(:value).with(:genmanifest).returns(true)
+                @settings.print_configs?.should be_true
+            end
+        end
+
+        describe "when printing configs" do
+            describe "when :configprint has a value" do
+                it "should call print_config_options" do
+                    @settings.stubs(:value).with(:configprint).returns("something")
+                    @settings.expects(:print_config_options)
+                    @settings.print_configs
+                end
+
+                it "should get the value of the option using the environment" do
+                    @settings.stubs(:value).with(:configprint).returns("something")
+                    @settings.stubs(:include?).with("something").returns(true)
+                    @settings.expects(:value).with(:environment).returns("env")
+                    @settings.expects(:value).with("something", "env").returns("foo")
+                    @settings.stubs(:puts).with("foo")
+                    @settings.print_configs
+                end
+
+                it "should print the value of the option" do
+                    @settings.stubs(:value).with(:configprint).returns("something")
+                    @settings.stubs(:include?).with("something").returns(true)
+                    @settings.stubs(:value).with("something", nil).returns("foo")
+                    @settings.expects(:puts).with("foo")
+                    @settings.print_configs
+                end
+
+                it "should print the value pairs if there are multiple options" do
+                    @settings.stubs(:value).with(:configprint).returns("bar,baz")
+                    @settings.stubs(:include?).with("bar").returns(true)
+                    @settings.stubs(:include?).with("baz").returns(true)
+                    @settings.stubs(:value).with("bar", nil).returns("foo")
+                    @settings.stubs(:value).with("baz", nil).returns("fud")
+                    @settings.expects(:puts).with("bar = foo")
+                    @settings.expects(:puts).with("baz = fud")
+                    @settings.print_configs
+                end
+
+                it "should print a whole bunch of stuff if :configprint = all"
+
+                it "should return true after printing" do
+                    @settings.stubs(:value).with(:configprint).returns("something")
+                    @settings.stubs(:include?).with("something").returns(true)
+                    @settings.stubs(:value).with("something", nil).returns("foo")
+                    @settings.stubs(:puts).with("foo")
+                    @settings.print_configs.should be_true
+                end
+
+                it "should return false if a config param is not found" do
+                    @settings.stubs(:value).with(:configprint).returns("something")
+                    @settings.stubs(:include?).with("something").returns(false)
+                    @settings.print_configs.should be_false
+                end
+            end
+
+            describe "when genconfig is true" do
+                it "should call to_config" do
+                    @settings.stubs(:value).with(:genconfig).returns(true)
+                    @settings.expects(:to_config)
+                    @settings.print_configs
+                end
+
+                it "should return true from print_configs" do
+                    @settings.stubs(:value).with(:genconfig).returns(true)
+                    @settings.stubs(:to_config)
+                    @settings.print_configs.should be_true
+                end
+            end
+
+            describe "when genmanifest is true" do
+                it "should call to_config" do
+                    @settings.stubs(:value).with(:genmanifest).returns(true)
+                    @settings.expects(:to_manifest)
+                    @settings.print_configs
+                end
+
+                it "should return true from print_configs" do
+                    @settings.stubs(:value).with(:genmanifest).returns(true)
+                    @settings.stubs(:to_manifest)
+                    @settings.print_configs.should be_true
+                end
+            end
+        end
+    end
 end
-- 
cgit 


From aedfa2bc06278d7bb3618cd39ea04f8aa8a9d846 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Mon, 16 Jun 2008 11:36:21 -0500
Subject: Fixed #1369 - the init service provider now supports HP-UX.

I've only added an integration test for the provider,
since that's all I've changed; none of the service providers
have rspec tests yet.
---
 CHANGELOG                                 |  3 +++
 lib/puppet/provider/service/init.rb       |  2 ++
 spec/integration/provider/service/init.rb | 32 +++++++++++++++++++++++++++++++
 3 files changed, 37 insertions(+)
 create mode 100755 spec/integration/provider/service/init.rb

diff --git a/CHANGELOG b/CHANGELOG
index 8140cfb6c..8286cc63d 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+0.24.?
+    Fixed #1369 - the init service provider now supports HP-UX.
+
     Removed support for the 'node_name' setting in LDAP and external node 
     lookups.
 
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/spec/integration/provider/service/init.rb b/spec/integration/provider/service/init.rb
new file mode 100755
index 000000000..e185247cb
--- /dev/null
+++ b/spec/integration/provider/service/init.rb
@@ -0,0 +1,32 @@
+#!/usr/bin/env ruby
+
+# Find and load the spec file.
+Dir.chdir(File.dirname(__FILE__)) { (s = lambda { |f| File.exist?(f) ? require(f) : Dir.chdir("..") { s.call(f) } }).call("spec/spec_helper.rb") }
+
+provider = Puppet::Type.type(:service).provider(:init)
+
+describe provider do
+    describe "when running on FreeBSD" do
+        confine "Not running on FreeBSD" => (Facter.value(:operatingsystem) == "FreeBSD")
+
+        it "should set its default path to include /etc/init.d and /usr/local/etc/init.d" do
+            provider.defpath.should == ["/etc/rc.d", "/usr/local/etc/rc.d"]
+        end
+    end
+
+    describe "when running on HP-UX" do
+        confine "Not running on HP-UX" => (Facter.value(:operatingsystem) == "HP-UX")
+
+        it "should set its default path to include /sbin/init.d" do
+            provider.defpath.should == "/sbin/init.d"
+        end
+    end
+
+    describe "when not running on FreeBSD or HP-UX" do
+        confine "Running on HP-UX or FreeBSD" => (! %w{HP-UX FreeBSD}.include?(Facter.value(:operatingsystem)))
+
+        it "should set its default path to include /etc/init.d" do
+            provider.defpath.should == "/etc/init.d"
+        end
+    end
+end
-- 
cgit 


From 9c1ab147b3570003bb7907d8a0cadca52869f4bd Mon Sep 17 00:00:00 2001
From: James Turnbull <james@lovedthanlost.net>
Date: Tue, 17 Jun 2008 09:43:45 +1000
Subject: Fixed #1371 - Updated bin/puppet to use Node.find

---
 bin/puppet | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bin/puppet b/bin/puppet
index 23b175087..5e10ad460 100755
--- a/bin/puppet
+++ b/bin/puppet
@@ -187,7 +187,7 @@ facts = Puppet::Node::Facts.find("me")
 facts.name = facts.values["hostname"]
 
 # Find our Node
-node = Puppet::Node.find_by_any_name(facts.name)
+node = Puppet::Node.find(facts.name)
 
 # Merge in the facts.
 node.merge(facts.values)
-- 
cgit 


From 955a8ff876e6476dd88b3502a8fa958180d0e0da Mon Sep 17 00:00:00 2001
From: James Turnbull <james@lovedthanlost.net>
Date: Tue, 17 Jun 2008 09:46:47 +1000
Subject: Removed test/util/loadedfile.rb tests which fixes #1370

---
 test/util/loadedfile.rb | 121 ------------------------------------------------
 1 file changed, 121 deletions(-)
 delete mode 100755 test/util/loadedfile.rb

diff --git a/test/util/loadedfile.rb b/test/util/loadedfile.rb
deleted file mode 100755
index fb640466a..000000000
--- a/test/util/loadedfile.rb
+++ /dev/null
@@ -1,121 +0,0 @@
-#!/usr/bin/env ruby
-
-require File.dirname(__FILE__) + '/../lib/puppettest'
-
-require 'puppet'
-require 'puppet/util/loadedfile'
-require 'puppettest'
-
-class TestLoadedFile < Test::Unit::TestCase
-	include PuppetTest
-    def test_file
-        Puppet[:filetimeout] = 0
-        file = nil
-        path = tempfile()
-        File.open(path, "w") { |f| f.puts "yayness" }
-        assert_nothing_raised {
-            file = Puppet::Util::LoadedFile.new(path)
-        }
-
-        assert(!file.changed?, "File incorrectly returned changed")
-
-        File.open(path, "w") { |f| f.puts "booness" }
-        #file.tstamp = File.stat(path).ctime - 5
-        new = File.stat(path).ctime - 5
-        file.tstamp = new
-
-        assert(file.changed?, "File did not catch change")
-    end
-
-    def test_timeout
-        Puppet[:filetimeout] = 50
-        path = tempfile()
-
-        File.open(path, "w") { |f| f.puts "yay" }
-        file = nil
-        assert_nothing_raised {
-            file = Puppet::Util::LoadedFile.new(path)
-        }
-
-        assert_nothing_raised {
-            assert(!file.changed?,
-                "File thought it changed immediately")
-        }
-
-        sleep 1
-        File.open(path, "w") { |f| f.puts "yay" }
-        #file.tstamp = File.stat(path).ctime - 5
-
-        assert(!file.changed?,
-            "File was marked as changed too soon")
-
-        Puppet[:filetimeout] = 0
-        assert(file.changed?,
-            "File was not marked as changed soon enough")
-    end
-
-    def test_stamp
-        file = tempfile()
-        File.open(file, "w") { |f| f.puts "" }
-        obj = nil
-        assert_nothing_raised {
-            obj = Puppet::Util::LoadedFile.new(file)
-        }
-
-        # Make sure we don't refresh
-        Puppet[:filetimeout] = 50
-
-        stamp = File.stat(file).ctime
-
-        assert_equal(stamp, obj.stamp)
-
-        sleep 1
-        # Now change the file, and make sure the stamp doesn't update yet
-        File.open(file, "w") { |f| f.puts "" }
-        assert_equal(stamp, obj.stamp,
-            "File prematurely refreshed")
-
-        Puppet[:filetimeout] = 0
-        assert_equal(File.stat(file).ctime, obj.stamp,
-            "File did not refresh")
-    end
-
-    # Testing #394.
-    def test_changed_missing_file
-        file = tempfile()
-        File.open(file, "w") { |f| f.puts "" }
-        obj = nil
-        assert_nothing_raised {
-            obj = Puppet::Util::LoadedFile.new(file)
-        }
-        Puppet[:filetimeout] = -10
-
-        assert_nothing_raised {
-            obj.changed?
-        }
-
-        # Now remove the file
-        File.unlink(file)
-
-        assert_nothing_raised("removed file threw an error") {
-            assert(obj.changed?, "File was not considered changed when missing")
-        }
-    end
-
-    # Make sure negative values always result in change notifications.
-    def test_negative_always_changes
-        file = tempfile()
-        File.open(file, "w") { |f| f.puts "" }
-        obj = nil
-        assert_nothing_raised {
-            obj = Puppet::Util::LoadedFile.new(file)
-        }
-
-        assert(! obj.changed?, "file with no change considered changed")
-        # Now set a negative value
-        Puppet[:filetimeout] = -1
-
-        assert(obj.changed?, "negative file timeout did not disable checking")
-    end
-end
-
-- 
cgit 


From 24ca81fcdca9fa321a706453c96536e9bbeab7e2 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Mon, 16 Jun 2008 22:42:12 -0500
Subject: Fixed #1360 -- allowdupe works with groups again.

I've added a couple of tests for this bit of the user
and group useradd/groupadd providers, but I haven't migrated
the rest of the tests.
---
 CHANGELOG                                    |  2 ++
 lib/puppet/provider/group/groupadd.rb        |  2 +-
 lib/puppet/provider/nameservice.rb           |  3 +--
 lib/puppet/provider/nameservice/objectadd.rb |  7 ++-----
 lib/puppet/provider/user/useradd.rb          |  2 +-
 spec/unit/provider/group/groupadd.rb         | 31 ++++++++++++++++++++++++++++
 spec/unit/provider/user/useradd.rb           | 31 ++++++++++++++++++++++++++++
 7 files changed, 69 insertions(+), 9 deletions(-)
 create mode 100755 spec/unit/provider/group/groupadd.rb
 create mode 100755 spec/unit/provider/user/useradd.rb

diff --git a/CHANGELOG b/CHANGELOG
index 8286cc63d..470071a1b 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
 0.24.?
+    Fixed #1360 - allowdupe works on groups again.
+
     Fixed #1369 - the init service provider now supports HP-UX.
 
     Removed support for the 'node_name' setting in LDAP and external node 
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/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/spec/unit/provider/group/groupadd.rb b/spec/unit/provider/group/groupadd.rb
new file mode 100755
index 000000000..fb4b811cb
--- /dev/null
+++ b/spec/unit/provider/group/groupadd.rb
@@ -0,0 +1,31 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+provider_class = Puppet::Type.type(:group).provider(:groupadd)
+
+describe provider_class do
+    before do
+        @resource = stub("resource", :name => "mygroup")
+        @provider = provider_class.new(@resource)
+    end
+
+    # #1360
+    it "should add -o when allowdupe is enabled and the group is being created" do
+        @resource.stubs(:should).returns "fakeval"
+        @resource.stubs(:[]).returns "fakeval"
+        @resource.expects(:allowdupe?).returns true
+        @provider.expects(:execute).with { |args| args.include?("-o") }
+
+        @provider.create
+    end
+
+    it "should add -o when allowdupe is enabled and the gid is being modified" do
+        @resource.stubs(:should).returns "fakeval"
+        @resource.stubs(:[]).returns "fakeval"
+        @resource.expects(:allowdupe?).returns true
+        @provider.expects(:execute).with { |args| args.include?("-o") }
+
+        @provider.gid = 150
+    end
+end
diff --git a/spec/unit/provider/user/useradd.rb b/spec/unit/provider/user/useradd.rb
new file mode 100755
index 000000000..96a785589
--- /dev/null
+++ b/spec/unit/provider/user/useradd.rb
@@ -0,0 +1,31 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../../spec_helper'
+
+provider_class = Puppet::Type.type(:user).provider(:useradd)
+
+describe provider_class do
+    before do
+        @resource = stub("resource", :name => "myuser", :managehome? => nil)
+        @provider = provider_class.new(@resource)
+    end
+
+    # #1360
+    it "should add -o when allowdupe is enabled and the user is being created" do
+        @resource.stubs(:should).returns "fakeval"
+        @resource.stubs(:[]).returns "fakeval"
+        @resource.expects(:allowdupe?).returns true
+        @provider.expects(:execute).with { |args| args.include?("-o") }
+
+        @provider.create
+    end
+
+    it "should add -o when allowdupe is enabled and the uid is being modified" do
+        @resource.stubs(:should).returns "fakeval"
+        @resource.stubs(:[]).returns "fakeval"
+        @resource.expects(:allowdupe?).returns true
+        @provider.expects(:execute).with { |args| args.include?("-o") }
+
+        @provider.uid = 150
+    end
+end
-- 
cgit 


From 4d9536418d8f684923063eac03d3a59d69bb3794 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Mon, 16 Jun 2008 23:41:33 -0500
Subject: Fixed #1221 - aliases to titles now work for resources.

---
 CHANGELOG                  |  2 ++
 lib/puppet/node/catalog.rb |  7 ++++++-
 spec/unit/node/catalog.rb  | 11 ++++++++---
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 470071a1b..8c0007415 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
 0.24.?
+    Fixed #1221 - aliases to titles now work for resources.
+
     Fixed #1360 - allowdupe works on groups again.
 
     Fixed #1369 - the init service provider now supports HP-UX.
diff --git a/lib/puppet/node/catalog.rb b/lib/puppet/node/catalog.rb
index 7d9a83795..720b3752d 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/spec/unit/node/catalog.rb b/spec/unit/node/catalog.rb
index ff9ab6930..b4f1503da 100755
--- a/spec/unit/node/catalog.rb
+++ b/spec/unit/node/catalog.rb
@@ -345,9 +345,9 @@ end
 describe Puppet::Node::Catalog, " when functioning as a resource container" do
     before do
         @catalog = Puppet::Node::Catalog.new("host")
-        @one = stub 'resource1', :ref => "Me[one]", :catalog= => nil
-        @two = stub 'resource2', :ref => "Me[two]", :catalog= => nil
-        @dupe = stub 'resource3', :ref => "Me[one]", :catalog= => nil
+        @one = stub 'resource1', :ref => "Me[one]", :catalog= => nil, :title => "one"
+        @two = stub 'resource2', :ref => "Me[two]", :catalog= => nil, :title => "two"
+        @dupe = stub 'resource3', :ref => "Me[one]", :catalog= => nil, :title => "one"
     end
 
     it "should provide a method to add one or more resources" do
@@ -482,6 +482,11 @@ describe Puppet::Node::Catalog, " when functioning as a resource container" do
         proc { @catalog.alias @one, "one" }.should_not raise_error
     end
 
+    it "should not create aliases that point back to the resource" do
+        @catalog.alias(@one, "one")
+        @catalog.resource(:me, "one").should be_nil
+    end
+
     it "should be able to look resources up by their aliases" do
         @catalog.add_resource @one
         @catalog.alias @one, "two"
-- 
cgit 


From 2380fcd4d187e8592398015e96605ce4b5d63e7d Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Mon, 16 Jun 2008 23:59:18 -0500
Subject: Fixed #1012 - templates in the templatedir are preferred to module
 templates.

---
 CHANGELOG            |  2 ++
 lib/puppet/module.rb |  6 +++++-
 spec/unit/module.rb  | 13 +++++++++++--
 3 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 470071a1b..f6c332db1 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
 0.24.?
+    Fixed #1012 - templates in the templatedir are preferred to module templates.
+
     Fixed #1360 - allowdupe works on groups again.
 
     Fixed #1369 - the init service provider now supports HP-UX.
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/spec/unit/module.rb b/spec/unit/module.rb
index 06b2d016d..2dadaa501 100755
--- a/spec/unit/module.rb
+++ b/spec/unit/module.rb
@@ -80,6 +80,14 @@ describe Puppet::Module, " when searching for templates" do
         File.stubs(:directory?).returns(true)
         Puppet::Module.find_template("mymod/mytemplate").should == "/one/mymod/templates/mytemplate"
     end
+    
+    it "should return the file in the templatedir if it exists" do
+        Puppet.settings.expects(:value).with(:templatedir, nil).returns("/my/templates")
+        Puppet[:modulepath] = "/one:/two"
+        File.stubs(:directory?).returns(true)
+        File.stubs(:exists?).returns(true)
+        Puppet::Module.find_template("mymod/mytemplate").should == "/my/templates/mymod/mytemplate"
+    end
 
     it "should use the main templatedir if no module is found" do
         Puppet.settings.expects(:value).with(:templatedir, nil).returns("/my/templates")
@@ -100,9 +108,10 @@ describe Puppet::Module, " when searching for templates" do
     end
 
     it "should use the node environment if specified" do
-        Puppet.settings.expects(:value).with(:modulepath, "myenv").returns("/my/templates")
+        Puppet.settings.stubs(:value).returns.returns("/my/directory")
+        Puppet.settings.expects(:value).with(:modulepath, "myenv").returns("/my/modules")
         File.stubs(:directory?).returns(true)
-        Puppet::Module.find_template("mymod/envtemplate", "myenv").should == "/my/templates/mymod/templates/envtemplate"
+        Puppet::Module.find_template("mymod/envtemplate", "myenv").should == "/my/modules/mymod/templates/envtemplate"
     end
 
     after { Puppet.settings.clear }
-- 
cgit 


From c83b23d029c8791c2369336702c6bf2ece5e2587 Mon Sep 17 00:00:00 2001
From: James Turnbull <james@lovedthanlost.net>
Date: Tue, 17 Jun 2008 16:56:10 +1000
Subject: Updated CHANGELOG for two missed commits

---
 CHANGELOG | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/CHANGELOG b/CHANGELOG
index 470071a1b..69cf7be17 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,8 @@
 0.24.?
+    Fixed #1367 - Updated Rakefile for new daily builds
+
+    Fixed #1370 - removed test/util/loadedfile.rb tests
+
     Fixed #1360 - allowdupe works on groups again.
 
     Fixed #1369 - the init service provider now supports HP-UX.
-- 
cgit 


From 00182ff96f18b54aa659a1909c23ba1aba253cd8 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 17 Jun 2008 00:53:30 -0500
Subject: Fixed #707 - special '@reboot'-style cron jobs work again.

---
 CHANGELOG                           | 2 ++
 lib/puppet/provider/cron/crontab.rb | 6 +++++-
 test/ral/providers/cron/crontab.rb  | 9 +++++----
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index bbf35de19..4a7e3c93c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -7,6 +7,8 @@
     
     Fixed #1012 - templates in the templatedir are preferred to module templates.
 
+    Fixed #707 - special '@reboot'-style cron jobs work again.
+
     Fixed #1360 - allowdupe works on groups again.
 
     Fixed #1369 - the init service provider now supports HP-UX.
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/test/ral/providers/cron/crontab.rb b/test/ral/providers/cron/crontab.rb
index 900a0478e..d9f460258 100755
--- a/test/ral/providers/cron/crontab.rb
+++ b/test/ral/providers/cron/crontab.rb
@@ -530,8 +530,6 @@ class TestCronParsedProvider < Test::Unit::TestCase
             @provider.initvars
             str += "\n"
             target.write(str)
-            assert_equal(str, target.read,
-                "Did not write correctly")
             assert_nothing_raised("Could not prefetch with %s" % str.inspect) do
                 @provider.prefetch
             end
@@ -551,6 +549,11 @@ class TestCronParsedProvider < Test::Unit::TestCase
         end
     end
 
+    # #707
+    def test_write_freebsd_special
+        assert_equal(@provider.to_line(:record_type => :crontab, :ensure => :present, :special => "reboot", :command => "/bin/echo something"), "@reboot /bin/echo something")
+    end
+
     def test_prefetch
         cron = @type.create :command => "/bin/echo yay", :name => "test", :hour => 4
 
@@ -626,6 +629,4 @@ class TestCronParsedProvider < Test::Unit::TestCase
             assert_equal(v, is[p], "did not parse %s correctly" % p)
         end
     end
-
 end
-
-- 
cgit 


From 17afb8afa0f1c3c58f2947f62938d262ae91b3c1 Mon Sep 17 00:00:00 2001
From: James Turnbull <james@lovedthanlost.net>
Date: Tue, 17 Jun 2008 22:10:09 +1000
Subject: Fixes #1195 - Updated Gentoo init scripts

---
 CHANGELOG                       |  2 ++
 conf/gentoo/init.d/puppet       |  2 +-
 conf/gentoo/init.d/puppetmaster | 68 ++++++++++++++++++++---------------------
 3 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 4a7e3c93c..a87702a10 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,6 @@
 0.24.?
+    Fixed #1195 - Updated Gentoo init scripts
+
     Fixed #1367 - Updated Rakefile for new daily builds
 
     Fixed #1370 - removed test/util/loadedfile.rb tests
diff --git a/conf/gentoo/init.d/puppet b/conf/gentoo/init.d/puppet
index 2fcd16943..83d30b024 100644
--- a/conf/gentoo/init.d/puppet
+++ b/conf/gentoo/init.d/puppet
@@ -24,7 +24,7 @@ start() {
         [[ -n "${PUPPET_EXTRA_OPTS}" ]] && options="${options} ${PUPPET_EXTRA_OPTS}"
 
         ebegin "Starting puppet"
-        start-stop-daemon --start --quiet --exec /usr/bin/puppetd -- ${options}
+        start-stop-daemon --start --quiet --exec /usr/bin/ruby /usr/bin/puppetd -- ${options}
         eend $? "Failed to start puppet"
 }
 
diff --git a/conf/gentoo/init.d/puppetmaster b/conf/gentoo/init.d/puppetmaster
index fcf71add4..438ac21e8 100755
--- a/conf/gentoo/init.d/puppetmaster
+++ b/conf/gentoo/init.d/puppetmaster
@@ -1,51 +1,51 @@
 #!/sbin/runscript
-# Copyright 1999-2004 Gentoo Foundation
+# Copyright 1999-2008 Gentoo Foundation
 # Distributed under the terms of the GNU General Public License v2
-# $Header$
 
 depend() {
-        need net
-        use dns logger
+	need net
+	before puppet
+	use dns logger
 }
 
 checkconfig() {
-        if [[ ! -d "${PUPPETMASTER_PID_DIR}" ]] ; then
-               eerror "Please make sure PUPPETMASTER_PID_DIR is defined and points to a existing directory"
-               return 1
-        fi
+	if [[ ! -d "${PUPPETMASTER_PID_DIR}" ]] ; then
+		eerror "Please make sure PUPPETMASTER_PID_DIR is defined and points to a existing directory"
+		return 1
+	fi
 
-        local site_manifest="/etc/puppet/manifests/site.pp"
-        [[ -n "${PUPPETMASTER_MANIFEST}" ]] && site_manifest="${PUPPETMASTER_MANIFEST}"
+	local site_manifest="/etc/puppet/manifests/site.pp"
+	[[ -n "${PUPPETMASTER_MANIFEST}" ]] && site_manifest="${PUPPETMASTER_MANIFEST}"
 
-        if [ ! -f "${site_manifest}" ] ; then
-                eerror "Please create ${site_manifest} before running puppet"
-                return 1
-        fi
+	if [ ! -f "${site_manifest}" ] ; then
+		eerror "Please create ${site_manifest} before running puppet"
+		return 1
+	fi
 
-        return 0
+	return 0
 }
 
 start() {
-        checkconfig || return $?
-
-        local options=""
-        [[ -n "${PUPPETMASTER_MANIFEST}" ]]   && options="${options} --manifest=${PUPPETMASTER_MANIFEST}"
-        [[ -n "${PUPPETMASTER_LOG}" ]]        && options="${options} --logdest=${PUPPETMASTER_LOG}"
-        [[ -n "${PUPPETMASTER_EXTRA_OPTS}" ]] && options="${options} ${PUPPETMASTER_EXTRA_OPTS}"
-
-        ebegin "Starting puppetmaster"
-        start-stop-daemon --start --quiet \
-                --pidfile ${PUPPETMASTER_PID_DIR}/puppetmasterd.pid --exec /usr/bin/puppetmasterd \
-                -- ${options}
-        eend $? "Failed to start puppetmaster"
+	checkconfig || return $?
+
+	local options=""
+	[[ -n "${PUPPETMASTER_MANIFEST}" ]]   && options="${options} --manifest=${PUPPETMASTER_MANIFEST}"
+	[[ -n "${PUPPETMASTER_LOG}" ]]        && options="${options} --logdest=${PUPPETMASTER_LOG}"
+	[[ -n "${PUPPETMASTER_EXTRA_OPTS}" ]] && options="${options} ${PUPPETMASTER_EXTRA_OPTS}"
+
+	ebegin "Starting puppetmaster"
+	start-stop-daemon --start --quiet --exec /usr/bin/ruby /usr/bin/puppetmasterd \
+		-- ${options}
+	eend $? "Failed to start puppetmaster"
 }
 
 stop() {
-        ebegin "Stopping puppetmaster"
-        start-stop-daemon --stop --quiet \
-                --pidfile ${PUPPETMASTER_PID_DIR}/puppetmasterd.pid
-        local ret=$?
-        eend ${ret} "Failed to stop puppetmaster"
-        rm -f ${PUPPETMASTER_PID_DIR}/puppetmasterd.pid
-        return ${ret}
+	ebegin "Stopping puppetmaster"
+	start-stop-daemon --stop --quiet \
+		--pidfile ${PUPPETMASTER_PID_DIR}/puppetmasterd.pid
+	local ret=$?
+	eend ${ret} "Failed to stop puppetmaster"
+	rm -f ${PUPPETMASTER_PID_DIR}/puppetmasterd.pid
+	return ${ret}
 }
+
-- 
cgit 


From 5a283d644b68bd89dc3c016ffd062b3ba792c617 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 17 Jun 2008 16:18:05 -0500
Subject: Fixing #1374  - Using Puppet::Type.type() in tests

---
 spec/unit/type/exec.rb     |   6 +--
 spec/unit/type/package.rb  |  72 ++++++++++++++++----------------
 spec/unit/type/schedule.rb |  40 +++++++++---------
 spec/unit/type/service.rb  | 100 ++++++++++++++++++++++-----------------------
 4 files changed, 105 insertions(+), 113 deletions(-)

diff --git a/spec/unit/type/exec.rb b/spec/unit/type/exec.rb
index cf0e02929..c6c082dd5 100755
--- a/spec/unit/type/exec.rb
+++ b/spec/unit/type/exec.rb
@@ -2,8 +2,6 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-require 'puppet/type/exec'
-
 module ExecModuleTesting
     def create_resource(command, output, exitstatus)
         @user_name = 'some_user_name'
@@ -30,7 +28,7 @@ module ExecModuleTesting
     end
 end
 
-describe Puppet::Type::Exec, " when execing" do
+describe Puppet::Type.type(:exec), " when execing" do
     include ExecModuleTesting
 
     it "should use the 'run_and_capture' method to exec" do
@@ -69,7 +67,7 @@ describe Puppet::Type::Exec, " when execing" do
 end
 
 
-describe Puppet::Type::Exec, " when logoutput=>on_failure is set," do
+describe Puppet::Type.type(:exec), " when logoutput=>on_failure is set," do
     include ExecModuleTesting
 
     it "should log the output on failure" do
diff --git a/spec/unit/type/package.rb b/spec/unit/type/package.rb
index 335910c63..3ceb8e32b 100755
--- a/spec/unit/type/package.rb
+++ b/spec/unit/type/package.rb
@@ -2,103 +2,101 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-require 'puppet/type/package'
-
-describe Puppet::Type::Package do
+describe Puppet::Type.type(:package) do
     it "should have an :installable feature that requires the :install method" do
-        Puppet::Type::Package.provider_feature(:installable).methods.should == [:install]
+        Puppet::Type.type(:package).provider_feature(:installable).methods.should == [:install]
     end
 
     it "should have an :uninstallable feature that requires the :uninstall method" do
-        Puppet::Type::Package.provider_feature(:uninstallable).methods.should == [:uninstall]
+        Puppet::Type.type(:package).provider_feature(:uninstallable).methods.should == [:uninstall]
     end
 
     it "should have an :upgradeable feature that requires :update and :latest methods" do
-        Puppet::Type::Package.provider_feature(:upgradeable).methods.should == [:update, :latest]
+        Puppet::Type.type(:package).provider_feature(:upgradeable).methods.should == [:update, :latest]
     end
 
     it "should have a :purgeable feature that requires the :purge latest method" do
-        Puppet::Type::Package.provider_feature(:purgeable).methods.should == [:purge]
+        Puppet::Type.type(:package).provider_feature(:purgeable).methods.should == [:purge]
     end
 
     it "should have a :versionable feature" do
-        Puppet::Type::Package.provider_feature(:versionable).should_not be_nil
+        Puppet::Type.type(:package).provider_feature(:versionable).should_not be_nil
     end
 
     it "should default to being installed" do
-        pkg = Puppet::Type::Package.create(:name => "yay")
+        pkg = Puppet::Type.type(:package).create(:name => "yay")
         pkg.should(:ensure).should == :present
     end
 
-    after { Puppet::Type::Package.clear }
+    after { Puppet::Type.type(:package).clear }
 end
 
-describe Puppet::Type::Package, "when validating attributes" do
+describe Puppet::Type.type(:package), "when validating attributes" do
     [:name, :source, :instance, :status, :adminfile, :responsefile, :configfiles, :category, :platform, :root, :vendor, :description, :allowcdrom].each do |param|
         it "should have a #{param} parameter" do
-            Puppet::Type::Package.attrtype(param).should == :param
+            Puppet::Type.type(:package).attrtype(param).should == :param
         end
     end
 
     it "should have an ensure property" do
-        Puppet::Type::Package.attrtype(:ensure).should == :property
+        Puppet::Type.type(:package).attrtype(:ensure).should == :property
     end
 end
 
-describe Puppet::Type::Package, "when validating attribute values" do
+describe Puppet::Type.type(:package), "when validating attribute values" do
     before do
-        @provider = stub 'provider', :class => Puppet::Type::Package.defaultprovider, :clear => nil
-        Puppet::Type::Package.defaultprovider.expects(:new).returns(@provider)
+        @provider = stub 'provider', :class => Puppet::Type.type(:package).defaultprovider, :clear => nil
+        Puppet::Type.type(:package).defaultprovider.expects(:new).returns(@provider)
     end
 
     it "should support :present as a value to :ensure" do
-        Puppet::Type::Package.create(:name => "yay", :ensure => :present)
+        Puppet::Type.type(:package).create(:name => "yay", :ensure => :present)
     end
 
     it "should alias :installed to :present as a value to :ensure" do
-        pkg = Puppet::Type::Package.create(:name => "yay", :ensure => :installed)
+        pkg = Puppet::Type.type(:package).create(:name => "yay", :ensure => :installed)
         pkg.should(:ensure).should == :present
     end
 
     it "should support :absent as a value to :ensure" do
-        Puppet::Type::Package.create(:name => "yay", :ensure => :absent)
+        Puppet::Type.type(:package).create(:name => "yay", :ensure => :absent)
     end
 
     it "should support :purged as a value to :ensure if the provider has the :purgeable feature" do
         @provider.expects(:satisfies?).with(:purgeable).returns(true)
-        Puppet::Type::Package.create(:name => "yay", :ensure => :purged)
+        Puppet::Type.type(:package).create(:name => "yay", :ensure => :purged)
     end
 
     it "should not support :purged as a value to :ensure if the provider does not have the :purgeable feature" do
         @provider.expects(:satisfies?).with(:purgeable).returns(false)
-        proc { Puppet::Type::Package.create(:name => "yay", :ensure => :purged) }.should raise_error(Puppet::Error)
+        proc { Puppet::Type.type(:package).create(:name => "yay", :ensure => :purged) }.should raise_error(Puppet::Error)
     end
 
     it "should support :latest as a value to :ensure if the provider has the :upgradeable feature" do
         @provider.expects(:satisfies?).with(:upgradeable).returns(true)
-        Puppet::Type::Package.create(:name => "yay", :ensure => :latest)
+        Puppet::Type.type(:package).create(:name => "yay", :ensure => :latest)
     end
 
     it "should not support :latest as a value to :ensure if the provider does not have the :upgradeable feature" do
         @provider.expects(:satisfies?).with(:upgradeable).returns(false)
-        proc { Puppet::Type::Package.create(:name => "yay", :ensure => :latest) }.should raise_error(Puppet::Error)
+        proc { Puppet::Type.type(:package).create(:name => "yay", :ensure => :latest) }.should raise_error(Puppet::Error)
     end
 
     it "should support version numbers as a value to :ensure if the provider has the :versionable feature" do
         @provider.expects(:satisfies?).with(:versionable).returns(true)
-        Puppet::Type::Package.create(:name => "yay", :ensure => "1.0")
+        Puppet::Type.type(:package).create(:name => "yay", :ensure => "1.0")
     end
 
     it "should not support version numbers as a value to :ensure if the provider does not have the :versionable feature" do
         @provider.expects(:satisfies?).with(:versionable).returns(false)
-        proc { Puppet::Type::Package.create(:name => "yay", :ensure => "1.0") }.should raise_error(Puppet::Error)
+        proc { Puppet::Type.type(:package).create(:name => "yay", :ensure => "1.0") }.should raise_error(Puppet::Error)
     end
 
     it "should accept any string as an argument to :source" do
-        proc { Puppet::Type::Package.create(:name => "yay", :source => "stuff") }.should_not raise_error(Puppet::Error)
+        proc { Puppet::Type.type(:package).create(:name => "yay", :source => "stuff") }.should_not raise_error(Puppet::Error)
     end
 
-    after { Puppet::Type::Package.clear }
+    after { Puppet::Type.type(:package).clear }
 end
 
 module PackageEvaluationTesting
@@ -107,11 +105,11 @@ module PackageEvaluationTesting
     end
 end
 
-describe Puppet::Type::Package do
+describe Puppet::Type.type(:package) do
     before :each do
-        @provider = stub 'provider', :class => Puppet::Type::Package.defaultprovider, :clear => nil, :satisfies? => true, :name => :mock
-        Puppet::Type::Package.defaultprovider.stubs(:new).returns(@provider)
-        @package = Puppet::Type::Package.create(:name => "yay")
+        @provider = stub 'provider', :class => Puppet::Type.type(:package).defaultprovider, :clear => nil, :satisfies? => true, :name => :mock
+        Puppet::Type.type(:package).defaultprovider.stubs(:new).returns(@provider)
+        @package = Puppet::Type.type(:package).create(:name => "yay")
 
         @catalog = Puppet::Node::Catalog.new
         @catalog.add_resource(@package)
@@ -119,11 +117,11 @@ describe Puppet::Type::Package do
     
     after :each do
         @catalog.clear(true)
-        Puppet::Type::Package.clear
+        Puppet::Type.type(:package).clear
     end
 
 
-    describe Puppet::Type::Package, "when it should be purged" do
+    describe Puppet::Type.type(:package), "when it should be purged" do
         include PackageEvaluationTesting
 
         before { @package[:ensure] = :purged }
@@ -142,7 +140,7 @@ describe Puppet::Type::Package do
         end
     end
 
-    describe Puppet::Type::Package, "when it should be absent" do
+    describe Puppet::Type.type(:package), "when it should be absent" do
         include PackageEvaluationTesting
 
         before { @package[:ensure] = :absent }
@@ -163,7 +161,7 @@ describe Puppet::Type::Package do
         end
     end
 
-    describe Puppet::Type::Package, "when it should be present" do
+    describe Puppet::Type.type(:package), "when it should be present" do
         include PackageEvaluationTesting
 
         before { @package[:ensure] = :present }
@@ -184,7 +182,7 @@ describe Puppet::Type::Package do
         end
     end
 
-    describe Puppet::Type::Package, "when it should be latest" do
+    describe Puppet::Type.type(:package), "when it should be latest" do
         include PackageEvaluationTesting
 
         before { @package[:ensure] = :latest }
@@ -219,7 +217,7 @@ describe Puppet::Type::Package do
         end
     end
 
-    describe Puppet::Type::Package, "when it should be a specific version" do
+    describe Puppet::Type.type(:package), "when it should be a specific version" do
         include PackageEvaluationTesting
 
         before { @package[:ensure] = "1.0" }
diff --git a/spec/unit/type/schedule.rb b/spec/unit/type/schedule.rb
index da38f68a9..d5d4c6039 100755
--- a/spec/unit/type/schedule.rb
+++ b/spec/unit/type/schedule.rb
@@ -2,8 +2,6 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-require 'puppet/type/schedule'
-
 module ScheduleTesting
 
     def format(time)
@@ -41,20 +39,20 @@ module ScheduleTesting
 
 end
 
-describe Puppet::Type::Schedule do
+describe Puppet::Type.type(:schedule) do
 
     before :each do
       Puppet.settings.stubs(:value).with(:ignoreschedules).returns(false)
 
-      @schedule = Puppet::Type::Schedule.create(:name => "testing")
+      @schedule = Puppet::Type.type(:schedule).create(:name => "testing")
     end
 
     after :each do
-        Puppet::Type::Schedule.clear
+        Puppet::Type.type(:schedule).clear
     end
 
 
-    describe Puppet::Type::Schedule do
+    describe Puppet::Type.type(:schedule) do
         include ScheduleTesting
 
         it "should default to :distance for period-matching" do
@@ -71,26 +69,26 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when producing default schedules" do
+    describe Puppet::Type.type(:schedule), "when producing default schedules" do
         include ScheduleTesting
 
         %w{hourly daily weekly monthly never}.each do |period|
             period = period.to_sym
             it "should produce a #{period} schedule with the period set appropriately" do
-                schedules = Puppet::Type::Schedule.mkdefaultschedules
-                schedules.find { |s| s[:name] == period.to_s and s[:period] == period }.should be_instance_of(Puppet::Type::Schedule)
+                schedules = Puppet::Type.type(:schedule).mkdefaultschedules
+                schedules.find { |s| s[:name] == period.to_s and s[:period] == period }.should be_instance_of(Puppet::Type.type(:schedule))
             end
         end
 
         it "should produce a schedule named puppet with a period of hourly and a repeat of 2" do
-            schedules = Puppet::Type::Schedule.mkdefaultschedules
+            schedules = Puppet::Type.type(:schedule).mkdefaultschedules
             schedules.find { |s|
                 s[:name] == "puppet" and s[:period] == :hourly and s[:repeat] == 2
-            }.should be_instance_of(Puppet::Type::Schedule)
+            }.should be_instance_of(Puppet::Type.type(:schedule))
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching ranges" do
+    describe Puppet::Type.type(:schedule), "when matching ranges" do
         include ScheduleTesting
 
         it "should match when the start time is before the current time and the end time is after the current time" do
@@ -109,7 +107,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching hourly by distance" do
+    describe Puppet::Type.type(:schedule), "when matching hourly by distance" do
         include ScheduleTesting
 
         before do
@@ -130,7 +128,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching daily by distance" do
+    describe Puppet::Type.type(:schedule), "when matching daily by distance" do
         include ScheduleTesting
 
         before do
@@ -151,7 +149,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching weekly by distance" do
+    describe Puppet::Type.type(:schedule), "when matching weekly by distance" do
         include ScheduleTesting
 
         before do
@@ -172,7 +170,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching monthly by distance" do
+    describe Puppet::Type.type(:schedule), "when matching monthly by distance" do
         include ScheduleTesting
 
         before do
@@ -193,7 +191,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching hourly by number" do
+    describe Puppet::Type.type(:schedule), "when matching hourly by number" do
         include ScheduleTesting
 
         before do
@@ -226,7 +224,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching daily by number" do
+    describe Puppet::Type.type(:schedule), "when matching daily by number" do
         include ScheduleTesting
 
         before do
@@ -261,7 +259,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching weekly by number" do
+    describe Puppet::Type.type(:schedule), "when matching weekly by number" do
         include ScheduleTesting
 
         before do
@@ -288,7 +286,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching monthly by number" do
+    describe Puppet::Type.type(:schedule), "when matching monthly by number" do
         include ScheduleTesting
 
         before do
@@ -315,7 +313,7 @@ describe Puppet::Type::Schedule do
         end
     end
 
-    describe Puppet::Type::Schedule, "when matching with a repeat greater than one" do
+    describe Puppet::Type.type(:schedule), "when matching with a repeat greater than one" do
         include ScheduleTesting
 
         before do
diff --git a/spec/unit/type/service.rb b/spec/unit/type/service.rb
index e8358cb22..12dda3edb 100755
--- a/spec/unit/type/service.rb
+++ b/spec/unit/type/service.rb
@@ -2,159 +2,157 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-require 'puppet/type/service'
-
-describe Puppet::Type::Service do
+describe Puppet::Type.type(:service) do
     it "should have an :enableable feature that requires the :enable, :disable, and :enabled? methods" do
-        Puppet::Type::Service.provider_feature(:enableable).methods.should == [:disable, :enable, :enabled?]
+        Puppet::Type.type(:service).provider_feature(:enableable).methods.should == [:disable, :enable, :enabled?]
     end
 
     it "should have a :refreshable feature that requires the :restart method" do
-        Puppet::Type::Service.provider_feature(:refreshable).methods.should == [:restart]
+        Puppet::Type.type(:service).provider_feature(:refreshable).methods.should == [:restart]
     end
 end
 
-describe Puppet::Type::Service, "when validating attributes" do
+describe Puppet::Type.type(:service), "when validating attributes" do
     [:name, :binary, :hasstatus, :path, :pattern, :start, :restart, :stop, :status, :hasrestart, :control].each do |param|
         it "should have a #{param} parameter" do
-            Puppet::Type::Service.attrtype(param).should == :param
+            Puppet::Type.type(:service).attrtype(param).should == :param
         end
     end
 
     [:ensure, :enable].each do |param|
         it "should have an #{param} property" do
-            Puppet::Type::Service.attrtype(param).should == :property
+            Puppet::Type.type(:service).attrtype(param).should == :property
         end
     end
 end
 
-describe Puppet::Type::Service, "when validating attribute values" do
+describe Puppet::Type.type(:service), "when validating attribute values" do
     before do
-        @provider = stub 'provider', :class => Puppet::Type::Service.defaultprovider, :clear => nil, :controllable? => false
-        Puppet::Type::Service.defaultprovider.stubs(:new).returns(@provider)
+        @provider = stub 'provider', :class => Puppet::Type.type(:service).defaultprovider, :clear => nil, :controllable? => false
+        Puppet::Type.type(:service).defaultprovider.stubs(:new).returns(@provider)
     end
 
     it "should support :running as a value to :ensure" do
-        Puppet::Type::Service.create(:name => "yay", :ensure => :running)
+        Puppet::Type.type(:service).create(:name => "yay", :ensure => :running)
     end
 
     it "should support :stopped as a value to :ensure" do
-        Puppet::Type::Service.create(:name => "yay", :ensure => :stopped)
+        Puppet::Type.type(:service).create(:name => "yay", :ensure => :stopped)
     end
 
     it "should alias the value :true to :running in :ensure" do
-        svc = Puppet::Type::Service.create(:name => "yay", :ensure => true)
+        svc = Puppet::Type.type(:service).create(:name => "yay", :ensure => true)
         svc.should(:ensure).should == :running
     end
 
     it "should alias the value :false to :stopped in :ensure" do
-        svc = Puppet::Type::Service.create(:name => "yay", :ensure => false)
+        svc = Puppet::Type.type(:service).create(:name => "yay", :ensure => false)
         svc.should(:ensure).should == :stopped
     end
 
     it "should support :true as a value to :enable" do
-        Puppet::Type::Service.create(:name => "yay", :enable => :true)
+        Puppet::Type.type(:service).create(:name => "yay", :enable => :true)
     end
 
     it "should support :false as a value to :enable" do
-        Puppet::Type::Service.create(:name => "yay", :enable => :false)
+        Puppet::Type.type(:service).create(:name => "yay", :enable => :false)
     end
 
     it "should support :true as a value to :hasstatus" do
-        Puppet::Type::Service.create(:name => "yay", :hasstatus => :true)
+        Puppet::Type.type(:service).create(:name => "yay", :hasstatus => :true)
     end
 
     it "should support :false as a value to :hasstatus" do
-        Puppet::Type::Service.create(:name => "yay", :hasstatus => :false)
+        Puppet::Type.type(:service).create(:name => "yay", :hasstatus => :false)
     end
 
     it "should support :true as a value to :hasrestart" do
-        Puppet::Type::Service.create(:name => "yay", :hasrestart => :true)
+        Puppet::Type.type(:service).create(:name => "yay", :hasrestart => :true)
     end
 
     it "should support :false as a value to :hasrestart" do
-        Puppet::Type::Service.create(:name => "yay", :hasrestart => :false)
+        Puppet::Type.type(:service).create(:name => "yay", :hasrestart => :false)
     end
 
     it "should allow setting the :enable parameter if the provider has the :enableable feature" do
-        Puppet::Type::Service.defaultprovider.stubs(:supports_parameter?).returns(true)
-        Puppet::Type::Service.defaultprovider.expects(:supports_parameter?).with(Puppet::Type::Service.attrclass(:enable)).returns(true)
-        svc = Puppet::Type::Service.create(:name => "yay", :enable => true)
+        Puppet::Type.type(:service).defaultprovider.stubs(:supports_parameter?).returns(true)
+        Puppet::Type.type(:service).defaultprovider.expects(:supports_parameter?).with(Puppet::Type.type(:service).attrclass(:enable)).returns(true)
+        svc = Puppet::Type.type(:service).create(:name => "yay", :enable => true)
         svc.should(:enable).should == :true
     end
 
     it "should not allow setting the :enable parameter if the provider is missing the :enableable feature" do
-        Puppet::Type::Service.defaultprovider.stubs(:supports_parameter?).returns(true)
-        Puppet::Type::Service.defaultprovider.expects(:supports_parameter?).with(Puppet::Type::Service.attrclass(:enable)).returns(false)
-        svc = Puppet::Type::Service.create(:name => "yay", :enable => true)
+        Puppet::Type.type(:service).defaultprovider.stubs(:supports_parameter?).returns(true)
+        Puppet::Type.type(:service).defaultprovider.expects(:supports_parameter?).with(Puppet::Type.type(:service).attrclass(:enable)).returns(false)
+        svc = Puppet::Type.type(:service).create(:name => "yay", :enable => true)
         svc.should(:enable).should be_nil
     end
 
     it "should discard paths that do not exist" do
         FileTest.stubs(:exist?).returns(false)
         FileTest.stubs(:directory?).returns(false)
-        svc = Puppet::Type::Service.create(:name => "yay", :path => "/one/two")
+        svc = Puppet::Type.type(:service).create(:name => "yay", :path => "/one/two")
         svc[:path].should be_empty
     end
 
     it "should discard paths that are not directories" do
         FileTest.stubs(:exist?).returns(true)
         FileTest.stubs(:directory?).returns(false)
-        svc = Puppet::Type::Service.create(:name => "yay", :path => "/one/two")
+        svc = Puppet::Type.type(:service).create(:name => "yay", :path => "/one/two")
         svc[:path].should be_empty
     end
 
     it "should split paths on ':'" do
         FileTest.stubs(:exist?).returns(true)
         FileTest.stubs(:directory?).returns(true)
-        svc = Puppet::Type::Service.create(:name => "yay", :path => "/one/two:/three/four")
+        svc = Puppet::Type.type(:service).create(:name => "yay", :path => "/one/two:/three/four")
         svc[:path].should == %w{/one/two /three/four}
     end
 
     it "should accept arrays of paths joined by ':'" do
         FileTest.stubs(:exist?).returns(true)
         FileTest.stubs(:directory?).returns(true)
-        svc = Puppet::Type::Service.create(:name => "yay", :path => ["/one:/two", "/three:/four"])
+        svc = Puppet::Type.type(:service).create(:name => "yay", :path => ["/one:/two", "/three:/four"])
         svc[:path].should == %w{/one /two /three /four}
     end
 
-    after { Puppet::Type::Service.clear }
+    after { Puppet::Type.type(:service).clear }
 end
 
-describe Puppet::Type::Service, "when setting default attribute values" do
+describe Puppet::Type.type(:service), "when setting default attribute values" do
     it "should default to the provider's default path if one is available" do
         FileTest.stubs(:directory?).returns(true)
         FileTest.stubs(:exist?).returns(true)
 
-        Puppet::Type::Service.defaultprovider.stubs(:respond_to?).returns(true)
-        Puppet::Type::Service.defaultprovider.stubs(:defpath).returns("testing")
-        svc = Puppet::Type::Service.create(:name => "other")
+        Puppet::Type.type(:service).defaultprovider.stubs(:respond_to?).returns(true)
+        Puppet::Type.type(:service).defaultprovider.stubs(:defpath).returns("testing")
+        svc = Puppet::Type.type(:service).create(:name => "other")
         svc[:path].should == ["testing"]
     end
 
     it "should default 'pattern' to the binary if one is provided" do
-        svc = Puppet::Type::Service.create(:name => "other", :binary => "/some/binary")
+        svc = Puppet::Type.type(:service).create(:name => "other", :binary => "/some/binary")
         svc[:pattern].should == "/some/binary"
     end
 
     it "should default 'pattern' to the name if no pattern is provided" do
-        svc = Puppet::Type::Service.create(:name => "other")
+        svc = Puppet::Type.type(:service).create(:name => "other")
         svc[:pattern].should == "other"
     end
 
     it "should default 'control' to the upcased service name with periods replaced by underscores if the provider supports the 'controllable' feature" do
-        provider = stub 'provider', :controllable? => true, :class => Puppet::Type::Service.defaultprovider, :clear => nil
-        Puppet::Type::Service.defaultprovider.stubs(:new).returns(provider)
-        svc = Puppet::Type::Service.create(:name => "nfs.client")
+        provider = stub 'provider', :controllable? => true, :class => Puppet::Type.type(:service).defaultprovider, :clear => nil
+        Puppet::Type.type(:service).defaultprovider.stubs(:new).returns(provider)
+        svc = Puppet::Type.type(:service).create(:name => "nfs.client")
         svc[:control].should == "NFS_CLIENT_START"
     end
 
-    after { Puppet::Type::Service.clear }
+    after { Puppet::Type.type(:service).clear }
 end
 
-describe Puppet::Type::Service, "when retrieving the host's current state" do
+describe Puppet::Type.type(:service), "when retrieving the host's current state" do
     before do
-        @service = Puppet::Type::Service.create(:name => "yay")
+        @service = Puppet::Type.type(:service).create(:name => "yay")
     end
 
     it "should use the provider's status to determine whether the service is running" do
@@ -170,12 +168,12 @@ describe Puppet::Type::Service, "when retrieving the host's current state" do
         @service.property(:enable).retrieve.should == :yepper
     end
 
-    after { Puppet::Type::Service.clear }
+    after { Puppet::Type.type(:service).clear }
 end
 
-describe Puppet::Type::Service, "when changing the host" do
+describe Puppet::Type.type(:service), "when changing the host" do
     before do
-        @service = Puppet::Type::Service.create(:name => "yay")
+        @service = Puppet::Type.type(:service).create(:name => "yay")
     end
 
     it "should start the service if it is supposed to be running" do
@@ -218,12 +216,12 @@ describe Puppet::Type::Service, "when changing the host" do
         @service.property(:ensure).sync
     end
 
-    after { Puppet::Type::Service.clear }
+    after { Puppet::Type.type(:service).clear }
 end
 
-describe Puppet::Type::Service, "when refreshing the service" do
+describe Puppet::Type.type(:service), "when refreshing the service" do
     before do
-        @service = Puppet::Type::Service.create(:name => "yay")
+        @service = Puppet::Type.type(:service).create(:name => "yay")
     end
 
     it "should restart the service if it is running" do
@@ -252,5 +250,5 @@ describe Puppet::Type::Service, "when refreshing the service" do
         @service.refresh
     end
 
-    after { Puppet::Type::Service.clear }
+    after { Puppet::Type.type(:service).clear }
 end
-- 
cgit 


From f1d59034e8100d188b1cf4e5d6f5b6f71a4b91b9 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 1 Jul 2008 10:31:29 -0500
Subject: Fixing #1388 - the package test no longer uses 'require'.

---
 spec/integration/type/package.rb | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/spec/integration/type/package.rb b/spec/integration/type/package.rb
index c244fa1cd..def44ad97 100755
--- a/spec/integration/type/package.rb
+++ b/spec/integration/type/package.rb
@@ -2,12 +2,10 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
-require 'puppet/type/package'
-
-describe Puppet::Type::Package, "when choosing a default package provider" do
+describe Puppet::Type.type(:package), "when choosing a default package provider" do
     before do
         # the default provider is cached.
-        Puppet::Type::Package.defaultprovider = nil
+        Puppet::Type.type(:package).defaultprovider = nil
     end
 
     def provider_name(os)
@@ -15,10 +13,10 @@ describe Puppet::Type::Package, "when choosing a default package provider" do
     end
 
     it "should have a default provider" do
-        Puppet::Type::Package.defaultprovider.should_not be_nil
+        Puppet::Type.type(:package).defaultprovider.should_not be_nil
     end
 
     it "should choose the correct provider each platform" do
-        Puppet::Type::Package.defaultprovider.name.should == provider_name(Facter.value(:operatingsystem))
+        Puppet::Type.type(:package).defaultprovider.name.should == provider_name(Facter.value(:operatingsystem))
     end
 end
-- 
cgit 


From ee9d0025eeebda1522d450b5bd3cda769d7455d5 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 1 Jul 2008 12:57:56 -0500
Subject: Fixed #1114 - Facts in plugin directories should now be autoloaded,
 as long as you're using Facter 1.5.

---
 CHANGELOG                           |  3 +++
 lib/puppet/network/client/master.rb | 37 +++++++++++++++++--------------------
 test/network/client/master.rb       | 16 ++++++++++------
 3 files changed, 30 insertions(+), 26 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index a87702a10..ae490cf8f 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,7 @@
 0.24.?
+    Fixed #1114 - Facts in plugin directories should now be autoloaded,
+    as long as you're using Facter 1.5.
+
     Fixed #1195 - Updated Gentoo init scripts
 
     Fixed #1367 - Updated Rakefile for new daily builds
diff --git a/lib/puppet/network/client/master.rb b/lib/puppet/network/client/master.rb
index 22bb3fa4e..0de3a117b 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
@@ -115,6 +129,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
@@ -122,9 +139,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
@@ -340,23 +354,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/test/network/client/master.rb b/test/network/client/master.rb
index 9f71247fa..0c1405217 100755
--- a/test/network/client/master.rb
+++ b/test/network/client/master.rb
@@ -177,10 +177,12 @@ end
 
         assert(File.exists?(destfile), "Did not get fact")
 
-        assert_equal(hostname, Facter.value(:hostname),
+        facts = Puppet::Network::Client.master.facts
+
+        assert_equal(hostname, facts["hostname"],
             "Lost value to hostname")
 
-        assert_equal("yayness", Facter.value(:myfact),
+        assert_equal("yayness", facts["myfact"],
             "Did not get correct fact value")
 
         # Now modify the file and make sure the type is replaced
@@ -194,20 +196,22 @@ end
         assert_nothing_raised {
             Puppet::Network::Client.master.getfacts
         }
+        facts = Puppet::Network::Client.master.facts
 
-        assert_equal("funtest", Facter.value(:myfact),
+        assert_equal("funtest", facts["myfact"],
             "Did not reload fact")
-        assert_equal(hostname, Facter.value(:hostname),
+        assert_equal(hostname, facts["hostname"],
             "Lost value to hostname")
 
         # Now run it again and make sure the fact still loads
         assert_nothing_raised {
             Puppet::Network::Client.master.getfacts
         }
+        facts = Puppet::Network::Client.master.facts
 
-        assert_equal("funtest", Facter.value(:myfact),
+        assert_equal("funtest", facts["myfact"],
             "Did not reload fact")
-        assert_equal(hostname, Facter.value(:hostname),
+        assert_equal(hostname, facts["hostname"],
             "Lost value to hostname")
     end
 
-- 
cgit 


From a1d1abdd5a2fc11dceeed63da8c6f48d2fa21cfe Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 1 Jul 2008 21:20:13 -0500
Subject: Adding an 'instance' class method to ldap connections.

This just returns a Connection instance with the default
ldap configuration information already provided.
---
 lib/puppet/util/ldap/connection.rb | 13 ++++++++++++
 spec/unit/util/ldap/connection.rb  | 42 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

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/spec/unit/util/ldap/connection.rb b/spec/unit/util/ldap/connection.rb
index 212f3ca54..9392466c7 100755
--- a/spec/unit/util/ldap/connection.rb
+++ b/spec/unit/util/ldap/connection.rb
@@ -111,4 +111,46 @@ describe Puppet::Util::Ldap::Connection do
             @connection.close
         end
     end
+
+    it "should have a class-level method for creating a default connection" do
+        Puppet::Util::Ldap::Connection.should respond_to(:instance)
+    end
+
+    describe "when creating a default connection" do
+        before do
+            Puppet.settings.stubs(:value).returns "whatever"
+        end
+
+        it "should use the :ldapserver setting to determine the host" do
+            Puppet.settings.expects(:value).with(:ldapserver).returns "myserv"
+            Puppet::Util::Ldap::Connection.expects(:new).with { |host, port, options| host == "myserv" }
+            Puppet::Util::Ldap::Connection.instance
+        end
+
+        it "should use the :ldapport setting to determine the port" do
+            Puppet.settings.expects(:value).with(:ldapport).returns "456"
+            Puppet::Util::Ldap::Connection.expects(:new).with { |host, port, options| port == "456" }
+            Puppet::Util::Ldap::Connection.instance
+        end
+
+        it "should set ssl to :tls if tls is enabled" do
+            Puppet.settings.expects(:value).with(:ldaptls).returns true
+            Puppet::Util::Ldap::Connection.expects(:new).with { |host, port, options| options[:ssl] == :tls }
+            Puppet::Util::Ldap::Connection.instance
+        end
+
+        it "should set ssl to 'true' if ssl is enabled and tls is not" do
+            Puppet.settings.expects(:value).with(:ldaptls).returns false
+            Puppet.settings.expects(:value).with(:ldapssl).returns true
+            Puppet::Util::Ldap::Connection.expects(:new).with { |host, port, options| options[:ssl] == true }
+            Puppet::Util::Ldap::Connection.instance
+        end
+
+        it "should set ssl to false if neither ssl nor tls are enabled" do
+            Puppet.settings.expects(:value).with(:ldaptls).returns false
+            Puppet.settings.expects(:value).with(:ldapssl).returns false
+            Puppet::Util::Ldap::Connection.expects(:new).with { |host, port, options| options[:ssl] == false }
+            Puppet::Util::Ldap::Connection.instance
+        end
+    end
 end
-- 
cgit 


From b47d4e1b3e1224541e555648854baf0503b1612e Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 1 Jul 2008 21:55:53 -0500
Subject: Added a 'search' method to the ldap node terminus.

This makes it easy to find multiple nodes in ldap, and
was done so it can be used by puppetrun.
---
 lib/puppet/indirector/ldap.rb      |  19 +-
 lib/puppet/indirector/node/ldap.rb | 152 ++++++++-----
 spec/unit/indirector/ldap.rb       |  18 +-
 spec/unit/indirector/node/ldap.rb  | 439 +++++++++++++++++++------------------
 4 files changed, 333 insertions(+), 295 deletions(-)

diff --git a/lib/puppet/indirector/ldap.rb b/lib/puppet/indirector/ldap.rb
index 695d38a95..7c3aca0da 100644
--- a/lib/puppet/indirector/ldap.rb
+++ b/lib/puppet/indirector/ldap.rb
@@ -1,21 +1,14 @@
 require 'puppet/indirector/terminus'
 
 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 +28,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,7 +47,9 @@ 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
 
diff --git a/lib/puppet/indirector/node/ldap.rb b/lib/puppet/indirector/node/ldap.rb
index 4ed053eff..2f953bbcb 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(key, options = {})
+        if classes = 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/spec/unit/indirector/ldap.rb b/spec/unit/indirector/ldap.rb
index 2599bcecf..cf802d856 100755
--- a/spec/unit/indirector/ldap.rb
+++ b/spec/unit/indirector/ldap.rb
@@ -28,8 +28,9 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
         @request = stub 'request', :key => "yay"
     end
 
-    it "should call the ldapsearch method with the name being searched for" do
-        @searcher.expects(:ldapsearch).with("yay")
+    it "should call the ldapsearch method with the search filter" do
+        @searcher.expects(:search_filter).with("yay").returns("yay's filter")
+        @searcher.expects(:ldapsearch).with("yay's filter")
         @searcher.find @request
     end
 
@@ -61,15 +62,6 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
         @searcher.find @request
     end
 
-    it "should use the results of the :search_filter method as the search filter" do
-        @searcher.stubs(:search_filter).with("yay").returns("yay's filter")
-        @connection.expects(:search).with do |*args|
-            args[2].should == "yay's filter"
-            true
-        end
-        @searcher.find @request
-    end
-
     it "should use depth 2 when searching" do
         @connection.expects(:search).with do |*args|
             args[1].should == 2
@@ -80,7 +72,7 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
 
     it "should call process() on the first found entry" do
         @connection.expects(:search).yields("myresult")
-        @searcher.expects(:process).with("yay", "myresult")
+        @searcher.expects(:process).with("myresult")
         @searcher.find @request
     end
 
@@ -94,7 +86,7 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
                 raise "failed"
             end
         end.yields("myresult")
-        @searcher.expects(:process).with("yay", "myresult")
+        @searcher.expects(:process).with("myresult")
 
         @searcher.find @request
     end
diff --git a/spec/unit/indirector/node/ldap.rb b/spec/unit/indirector/node/ldap.rb
index 24b2dd759..03faaa557 100755
--- a/spec/unit/indirector/node/ldap.rb
+++ b/spec/unit/indirector/node/ldap.rb
@@ -5,304 +5,307 @@ require File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/indirector/node/ldap'
 
 describe Puppet::Node::Ldap do
-    describe "when searching for nodes" do
+    before do
+        Puppet::Node::Facts.stubs(:terminus_class).returns :yaml
+    end
+
+    describe "when searching for a single node" do
         before :each do
             @searcher = Puppet::Node::Ldap.new
-            @entries = {}
-            entries = @entries
             
-            @connection = mock 'connection'
-            @entry = mock 'entry'
-            @connection.stubs(:search).yields(@entry)
-            @searcher.stubs(:connection).returns(@connection)
-            @searcher.stubs(:class_attributes).returns([])
-            @searcher.stubs(:parent_attribute).returns(nil)
-            @searcher.stubs(:stacked_attributes).returns([])
-            @searcher.stubs(:search_base).returns(:yay)
-            @searcher.stubs(:search_filter).returns(:filter)
-
             @name = "mynode.domain.com"
-            @node = stub 'node', :name => @name
-            @node.stubs(:fact_merge)
-            Puppet::Node.stubs(:new).with(@name).returns(@node)
+            @node = stub 'node', :name => @name, :name= => nil
+            @node.stub_everything
+
+            Puppet::Node.stubs(:new).returns(@node)
 
             @request = stub 'request', :key => @name
         end
 
-        it "should search first for the provided key" do
-            @searcher.expects(:entry2hash).with("mynode.domain.com").returns({})
-            @searcher.find(@request)
+        it "should convert the hostname into a search filter" do
+            entry = stub 'entry', :dn => 'cn=mynode.domain.com,ou=hosts,dc=madstop,dc=com', :vals => %w{}, :to_hash => {}
+            @searcher.expects(:ldapsearch).with("(&(objectclass=puppetClient)(cn=#{@name}))").yields entry
+            @searcher.name2hash(@name)
         end
 
-        it "should search for the short version of the provided key if the key looks like a hostname and no results are found for the key itself" do
-            @searcher.expects(:entry2hash).with("mynode.domain.com").returns(nil)
-            @searcher.expects(:entry2hash).with("mynode").returns({})
-            @searcher.find(@request)
+        it "should convert any found entry into a hash" do
+            entry = stub 'entry', :dn => 'cn=mynode.domain.com,ou=hosts,dc=madstop,dc=com', :vals => %w{}, :to_hash => {}
+            @searcher.expects(:ldapsearch).with("(&(objectclass=puppetClient)(cn=#{@name}))").yields entry
+            @searcher.expects(:entry2hash).with(entry).returns "myhash"
+            @searcher.name2hash(@name).should == "myhash"
         end
 
-        it "should search for default information if no information can be found for the key" do
-            @searcher.expects(:entry2hash).with("mynode.domain.com").returns(nil)
-            @searcher.expects(:entry2hash).with("mynode").returns(nil)
-            @searcher.expects(:entry2hash).with("default").returns({})
-            @searcher.find(@request)
-        end
+        # 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 => {}
+                @searcher.stubs(:ldapsearch).yields @entry
+            end
 
-        it "should return nil if no results are found in ldap" do
-            @connection.stubs(:search)
-            @searcher.find(@request).should be_nil
-        end
+            it "should convert the entry to a hash" do
+                @searcher.entry2hash(@entry).should be_instance_of(Hash)
+            end
 
-        it "should return a node object if results are found in ldap" do
-            @entry.stubs(:to_hash).returns({})
-            @searcher.find(@request).should equal(@node)
-        end
+            it "should add the entry's common name to the hash" do
+                @searcher.entry2hash(@entry)[:name].should == "mynode.domain.com"
+            end
 
-        it "should deduplicate class values" do
-            @entry.stubs(:to_hash).returns({})
-            @searcher.stubs(:class_attributes).returns(%w{one two})
-            @entry.stubs(:vals).with("one").returns(%w{a b})
-            @entry.stubs(:vals).with("two").returns(%w{b c})
-            @node.expects(:classes=).with(%w{a b c})
-            @searcher.find(@request)
-        end
+            it "should add all of the entry's classes to the hash" do
+                @entry.stubs(:vals).with("puppetclass").returns %w{one two}
+                @searcher.entry2hash(@entry)[:classes].should == %w{one two}
+            end
 
-        it "should add any values stored in the class_attributes attributes to the node classes" do
-            @entry.stubs(:to_hash).returns({})
-            @searcher.stubs(:class_attributes).returns(%w{one two})
-            @entry.stubs(:vals).with("one").returns(%w{a b})
-            @entry.stubs(:vals).with("two").returns(%w{c d})
-            @node.expects(:classes=).with(%w{a b c d})
-            @searcher.find(@request)
+            it "should deduplicate class values" do
+                @entry.stubs(:to_hash).returns({})
+                @searcher.stubs(:class_attributes).returns(%w{one two})
+                @entry.stubs(:vals).with("one").returns(%w{a b})
+                @entry.stubs(:vals).with("two").returns(%w{b c})
+                @searcher.entry2hash(@entry)[:classes].should == %w{a b c}
+            end
+
+            it "should add the entry's environment to the hash" do
+                @entry.stubs(:to_hash).returns("environment" => %w{production})
+                @searcher.entry2hash(@entry)[:environment].should == "production"
+            end
+
+            it "should add all stacked parameters as parameters in the hash" do
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two three=four})
+                result = @searcher.entry2hash(@entry)
+                result[:parameters]["one"].should == "two"
+                result[:parameters]["three"].should == "four"
+            end
+
+            it "should not add the stacked parameter as a normal parameter" do
+                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two three=four})
+                @entry.stubs(:to_hash).returns("puppetvar" => %w{one=two three=four})
+                @searcher.entry2hash(@entry)[:parameters]["puppetvar"].should be_nil
+            end
+
+            it "should add all other attributes as parameters in the hash" do
+                @entry.stubs(:to_hash).returns("foo" => %w{one two})
+                @searcher.entry2hash(@entry)[:parameters]["foo"].should == %w{one two}
+            end
+
+            it "should return single-value parameters as strings, not arrays" do
+                @entry.stubs(:to_hash).returns("foo" => %w{one})
+                @searcher.entry2hash(@entry)[:parameters]["foo"].should == "one"
+            end
+
+            it "should add the parent's name if present" do
+                @entry.stubs(:vals).with("parentnode").returns(%w{foo})
+                @searcher.entry2hash(@entry)[:parent].should == "foo"
+            end
+
+            it "should fail if more than one parent is specified" do
+                @entry.stubs(:vals).with("parentnode").returns(%w{foo})
+                @searcher.entry2hash(@entry)[:parent].should == "foo"
+            end
         end
 
-        it "should add all entry attributes as node parameters" do
-            @entry.stubs(:to_hash).returns("one" => ["two"], "three" => ["four"])
-            @node.expects(:parameters=).with("one" => "two", "three" => "four")
+        it "should search first for the provided key" do
+            @searcher.expects(:name2hash).with("mynode.domain.com").returns({})
             @searcher.find(@request)
         end
 
-        it "should set the node's environment to the environment of the results" do
-            @entry.stubs(:to_hash).returns("environment" => ["test"])
-            @node.stubs(:parameters=)
-            @node.expects(:environment=).with("test")
+        it "should search for the short version of the provided key if the key looks like a hostname and no results are found for the key itself" do
+            @searcher.expects(:name2hash).with("mynode.domain.com").returns(nil)
+            @searcher.expects(:name2hash).with("mynode").returns({})
             @searcher.find(@request)
         end
 
-        it "should retain false parameter values" do
-            @entry.stubs(:to_hash).returns("one" => [false])
-            @node.expects(:parameters=).with("one" => false)
+        it "should search for default information if no information can be found for the key" do
+            @searcher.expects(:name2hash).with("mynode.domain.com").returns(nil)
+            @searcher.expects(:name2hash).with("mynode").returns(nil)
+            @searcher.expects(:name2hash).with("default").returns({})
             @searcher.find(@request)
         end
 
-        it "should turn single-value parameter value arrays into single non-arrays" do
-            @entry.stubs(:to_hash).returns("one" => ["a"])
-            @node.expects(:parameters=).with("one" => "a")
-            @searcher.find(@request)
+        it "should return nil if no results are found in ldap" do
+            @searcher.stubs(:name2hash).returns nil
+            @searcher.find(@request).should be_nil
         end
 
-        it "should keep multi-valued parametes as arrays" do
-            @entry.stubs(:to_hash).returns("one" => ["a", "b"])
-            @node.expects(:parameters=).with("one" => ["a", "b"])
-            @searcher.find(@request)
+        it "should return a node object if results are found in ldap" do
+            @searcher.stubs(:name2hash).returns({})
+            @searcher.find(@request).should equal(@node)
         end
 
-        describe "and a parent node is specified" do
+        describe "and node information is found in LDAP" do
             before do
-                @parent = mock 'parent'
-                @parent_parent = mock 'parent_parent'
-
-                @searcher.meta_def(:search_filter) do |name|
-                    return name
-                end
-                @connection.stubs(:search).with { |*args| args[2] == @name              }.yields(@entry)
-                @connection.stubs(:search).with { |*args| args[2] == 'parent'           }.yields(@parent)
-                @connection.stubs(:search).with { |*args| args[2] == 'parent_parent'    }.yields(@parent_parent)
-
-                @searcher.stubs(:parent_attribute).returns(:parent)
+                @result = {}
+                @searcher.stubs(:name2hash).returns @result
             end
 
-            it "should fail if the parent cannot be found" do
-                @connection.stubs(:search).with { |*args| args[2] == 'parent' }.returns("whatever")
-
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
-
-                proc { @searcher.find(@request) }.should raise_error(Puppet::Error)
-            end
-
-            it "should add any parent classes to the node's classes" do
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
-                @entry.stubs(:vals).with("classes").returns(%w{a b})
-
-                @parent.stubs(:to_hash).returns({})
-                @parent.stubs(:vals).with("classes").returns(%w{c d})
-                @parent.stubs(:vals).with(:parent).returns(nil)
-
-                @searcher.stubs(:class_attributes).returns(%w{classes})
+            it "should add any classes from ldap" do
+                @result[:classes] = %w[a b c d]
                 @node.expects(:classes=).with(%w{a b c d})
                 @searcher.find(@request)
             end
 
-            it "should add any parent parameters to the node's parameters" do
-                @entry.stubs(:to_hash).returns("one" => "two")
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
-
-                @parent.stubs(:to_hash).returns("three" => "four")
-                @parent.stubs(:vals).with(:parent).returns(nil)
-
+            it "should add all entry attributes as node parameters" do
+                @result[:parameters] = {"one" => "two", "three" => "four"}
                 @node.expects(:parameters=).with("one" => "two", "three" => "four")
                 @searcher.find(@request)
             end
 
-            it "should prefer node parameters over parent parameters" do
-                @entry.stubs(:to_hash).returns("one" => "two")
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
-
-                @parent.stubs(:to_hash).returns("one" => "three")
-                @parent.stubs(:vals).with(:parent).returns(nil)
+            it "should set the node's environment to the environment of the results" do
+                @result[:environment] = "test"
+                @node.expects(:environment=).with("test")
+                @searcher.find(@request)
+            end
 
-                @node.expects(:parameters=).with("one" => "two")
+            it "should retain false parameter values" do
+                @result[:parameters] = {}
+                @result[:parameters]["one"] = false
+                @node.expects(:parameters=).with("one" => false)
                 @searcher.find(@request)
             end
 
-            it "should use the parent's environment if the node has none" do
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+            describe "and a parent node is specified" do
+                before do
+                    @entry = {:classes => [], :parameters => {}}
+                    @parent = {:classes => [], :parameters => {}}
+                    @parent_parent = {:classes => [], :parameters => {}}
 
-                @parent.stubs(:to_hash).returns("environment" => ["parent"])
-                @parent.stubs(:vals).with(:parent).returns(nil)
+                    @searcher.stubs(:name2hash).with(@name).returns(@entry)
+                    @searcher.stubs(:name2hash).with('parent').returns(@parent)
+                    @searcher.stubs(:name2hash).with('parent_parent').returns(@parent_parent)
 
-                @node.stubs(:parameters=)
-                @node.expects(:environment=).with("parent")
-                @searcher.find(@request)
-            end
+                    @searcher.stubs(:parent_attribute).returns(:parent)
+                end
 
-            it "should prefer the node's environment to the parent's" do
-                @entry.stubs(:to_hash).returns("environment" => %w{child})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                it "should search for the parent node" do
+                    @entry[:parent] = "parent"
 
-                @parent.stubs(:to_hash).returns("environment" => ["parent"])
-                @parent.stubs(:vals).with(:parent).returns(nil)
+                    @searcher.expects(:name2hash).with(@name).returns @entry
+                    @searcher.expects(:name2hash).with('parent').returns @parent
 
-                @node.stubs(:parameters=)
-                @node.expects(:environment=).with("child")
-                @searcher.find(@request)
-            end
+                    @searcher.find(@request)
+                end
 
-            it "should recursively look up parent information" do
-                @entry.stubs(:to_hash).returns("one" => "two")
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                it "should fail if the parent cannot be found" do
+                    @entry[:parent] = "parent"
 
-                @parent.stubs(:to_hash).returns("three" => "four")
-                @parent.stubs(:vals).with(:parent).returns(['parent_parent'])
+                    @searcher.expects(:name2hash).with('parent').returns nil
 
-                @parent_parent.stubs(:to_hash).returns("five" => "six")
-                @parent_parent.stubs(:vals).with(:parent).returns(nil)
-                @parent_parent.stubs(:vals).with(:parent).returns(nil)
+                    proc { @searcher.find(@request) }.should raise_error(Puppet::Error)
+                end
 
-                @node.expects(:parameters=).with("one" => "two", "three" => "four", "five" => "six")
-                @searcher.find(@request)
-            end
+                it "should add any parent classes to the node's classes" do
+                    @entry[:parent] = "parent"
+                    @entry[:classes] = %w{a b}
 
-            it "should not allow loops in parent declarations" do
-                @entry.stubs(:to_hash).returns("one" => "two")
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                    @parent[:classes] = %w{c d}
 
-                @parent.stubs(:to_hash).returns("three" => "four")
-                @parent.stubs(:vals).with(:parent).returns([@name])
-                proc { @searcher.find(@request) }.should raise_error(ArgumentError)
-            end
-        end
+                    @node.expects(:classes=).with(%w{a b c d})
+                    @searcher.find(@request)
+                end
 
-        describe "and a puppet variable is specified" do
-            before do
-                @searcher.stubs(:stacked_attributes).returns(['puppetvar'])
-            end
+                it "should add any parent parameters to the node's parameters" do
+                    @entry[:parent] = "parent"
+                    @entry[:parameters]["one"] = "two"
 
-            it "should add the variable to the node parameters" do
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @entry.stubs(:to_hash).returns({})
-                @node.expects(:parameters=).with("one" => "two")
-                @searcher.find(@request)
-            end
+                    @parent[:parameters]["three"] = "four"
 
-            it "should not overwrite node parameters specified as ldap object attribute" do
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @entry.stubs(:to_hash).returns("one" => "three")
-                @node.expects(:parameters=).with("one" => "three")
-                @searcher.find(@request)
-            end
+                    @node.expects(:parameters=).with("one" => "two", "three" => "four")
+                    @searcher.find(@request)
+                end
 
-            it "should set entries without an equal sign to nil" do
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one})
-                @entry.stubs(:to_hash).returns({})
-                @node.expects(:parameters=).with("one" => nil)
-                @searcher.find(@request)
-            end
+                it "should prefer node parameters over parent parameters" do
+                    @entry[:parent] = "parent"
+                    @entry[:parameters]["one"] = "two"
 
-            it "should ignore empty entries" do
-                @entry.stubs(:vals).with("puppetvar").returns(%w{})
-                @entry.stubs(:to_hash).returns({})
-                @searcher.find(@request)
-            end
-        end
-        describe "and a puppet variable as well as a parent node are specified" do
-            before do
-                @parent = mock 'parent'
+                    @parent[:parameters]["one"] = "three"
 
-                @searcher.meta_def(:search_filter) do |name|
-                    return name
+                    @node.expects(:parameters=).with("one" => "two")
+                    @searcher.find(@request)
                 end
-                @connection.stubs(:search).with { |*args| args[2] == @name              }.yields(@entry)
-                @connection.stubs(:search).with { |*args| args[2] == 'parent'           }.yields(@parent)
 
-                @searcher.stubs(:stacked_attributes).returns(['puppetvar'])
-                @searcher.stubs(:parent_attribute).returns(:parent)
-            end
+                it "should use the parent's environment if the node has none" do
+                    @entry[:parent] = "parent"
 
-            it "should add parent node variables to the child node parameters" do
-                @parent.stubs(:to_hash).returns({})
-                @parent.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @parent.stubs(:vals).with(:parent).returns(nil)
+                    @parent[:environment] = "parent"
 
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with("puppetvar").returns(%w{})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                    @node.stubs(:parameters=)
+                    @node.expects(:environment=).with("parent")
+                    @searcher.find(@request)
+                end
 
-                @node.expects(:parameters=).with("one" => "two")
+                it "should prefer the node's environment to the parent's" do
+                    @entry[:parent] = "parent"
+                    @entry[:environment] = "child"
 
-                @searcher.find(@request)
-            end
+                    @parent[:environment] = "parent"
 
-            it "should overwrite parent node variables with child node parameters" do
-                @parent.stubs(:to_hash).returns({})
-                @parent.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @parent.stubs(:vals).with(:parent).returns(nil)
+                    @node.stubs(:parameters=)
+                    @node.expects(:environment=).with("child")
+                    @searcher.find(@request)
+                end
 
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one=three})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+                it "should recursively look up parent information" do
+                    @entry[:parent] = "parent"
+                    @entry[:parameters]["one"] = "two"
 
-                @node.expects(:parameters=).with("one" => "three")
+                    @parent[:parent] = "parent_parent"
+                    @parent[:parameters]["three"] = "four"
 
-                @searcher.find(@request)
+                    @parent_parent[:parameters]["five"] = "six"
+
+                    @node.expects(:parameters=).with("one" => "two", "three" => "four", "five" => "six")
+                    @searcher.find(@request)
+                end
+
+                it "should not allow loops in parent declarations" do
+                    @entry[:parent] = "parent"
+                    @parent[:parent] = @name
+                    proc { @searcher.find(@request) }.should raise_error(ArgumentError)
+                end
             end
+        end
+    end
 
-            it "should not overwrite parent node parameters specified as ldap object attribute" do
-                @parent.stubs(:to_hash).returns("one" => "three")
-                @parent.stubs(:vals).with("puppetvar").returns(%w{})
-                @parent.stubs(:vals).with(:parent).returns(nil)
+    describe "when searching for multiple nodes" do
+        before :each do
+            @searcher = Puppet::Node::Ldap.new
+            @request = stub 'request', :key => @name
 
-                @entry.stubs(:vals).with("puppetvar").returns(%w{one=two})
-                @entry.stubs(:to_hash).returns({})
-                @entry.stubs(:vals).with(:parent).returns(%w{parent})
+            Puppet::Node::Facts.stubs(:terminus_class).returns :yaml
+        end
 
-                @node.expects(:parameters=).with("one" => "three")
+        it "should find all nodes if no arguments are provided" 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"
+        end
 
-                @searcher.find(@request)
+        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"
+            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"]
             end
+        end
+
+        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.search "foo"
+        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[0].should be_instance_of(Puppet::Node)
+            result[0].name.should == "foo"
         end
     end
 end
-- 
cgit 


From 4d22a95b571991eb47046c3b0103b2e733b2801d Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 1 Jul 2008 22:06:40 -0500
Subject: Switching the ldap terminus to use Util::Ldap::Connection.

This is a simplified class for managing ldap connections,
and this work just removes some duplication.
---
 lib/puppet/indirector/ldap.rb |  19 ++---
 spec/unit/indirector/ldap.rb  | 186 ++++++++++++++++++++----------------------
 2 files changed, 94 insertions(+), 111 deletions(-)

diff --git a/lib/puppet/indirector/ldap.rb b/lib/puppet/indirector/ldap.rb
index 7c3aca0da..7485bd932 100644
--- a/lib/puppet/indirector/ldap.rb
+++ b/lib/puppet/indirector/ldap.rb
@@ -1,4 +1,5 @@
 require 'puppet/indirector/terminus'
+require 'puppet/util/ldap/connection'
 
 class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
     # Perform our ldap search and process the result.
@@ -56,8 +57,6 @@ class Puppet::Indirector::Ldap < Puppet::Indirector::Terminus
         return found
     end
 
-    private
-
     # Create an ldap connection.
     def connection
         unless defined? @connection and @connection
@@ -65,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/spec/unit/indirector/ldap.rb b/spec/unit/indirector/ldap.rb
index cf802d856..52abd413a 100755
--- a/spec/unit/indirector/ldap.rb
+++ b/spec/unit/indirector/ldap.rb
@@ -4,7 +4,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/indirector/ldap'
 
-describe Puppet::Indirector::Ldap, " when searching ldap" do
+describe Puppet::Indirector::Ldap do
     before do
         @indirection = stub 'indirection', :name => :testing
         Puppet::Indirector::Indirection.stubs(:instance).returns(@indirection)
@@ -17,125 +17,117 @@ describe Puppet::Indirector::Ldap, " when searching ldap" do
         @connection = mock 'ldap'
 
         @searcher = @ldap_class.new
-
-        # Stub everything, and we can selectively replace with an expect as
-        # we need to for testing.
-        @searcher.stubs(:connection).returns(@connection)
-        @searcher.stubs(:search_filter).returns(:filter)
-        @searcher.stubs(:search_base).returns(:base)
-        @searcher.stubs(:process)
-
-        @request = stub 'request', :key => "yay"
     end
 
-    it "should call the ldapsearch method with the search filter" do
-        @searcher.expects(:search_filter).with("yay").returns("yay's filter")
-        @searcher.expects(:ldapsearch).with("yay's filter")
-        @searcher.find @request
-    end
-
-    it "should fail if no block is passed to the ldapsearch method" do
-        proc { @searcher.ldapsearch("blah") }.should raise_error(ArgumentError)
-    end
+    describe "when searching ldap" do
+        before do
+            # Stub everything, and we can selectively replace with an expect as
+            # we need to for testing.
+            @searcher.stubs(:connection).returns(@connection)
+            @searcher.stubs(:search_filter).returns(:filter)
+            @searcher.stubs(:search_base).returns(:base)
+            @searcher.stubs(:process)
 
-    it "should use the results of the ldapbase method as the ldap search base" do
-        @searcher.stubs(:search_base).returns("mybase")
-        @connection.expects(:search).with do |*args|
-            args[0].should == "mybase"
-            true
+            @request = stub 'request', :key => "yay"
         end
-        @searcher.find @request
-    end
-
-    it "should default to the value of the :search_base setting as the result of the ldapbase method" do
-        Puppet.expects(:[]).with(:ldapbase).returns("myldapbase")
-        searcher = @ldap_class.new
-        searcher.search_base.should == "myldapbase"
-    end
 
-    it "should use the results of the :search_attributes method as the list of attributes to return" do
-        @searcher.stubs(:search_attributes).returns(:myattrs)
-        @connection.expects(:search).with do |*args|
-            args[3].should == :myattrs
-            true
+        it "should call the ldapsearch method with the search filter" do
+            @searcher.expects(:search_filter).with("yay").returns("yay's filter")
+            @searcher.expects(:ldapsearch).with("yay's filter")
+            @searcher.find @request
         end
-        @searcher.find @request
-    end
 
-    it "should use depth 2 when searching" do
-        @connection.expects(:search).with do |*args|
-            args[1].should == 2
-            true
+        it "should fail if no block is passed to the ldapsearch method" do
+            proc { @searcher.ldapsearch("blah") }.should raise_error(ArgumentError)
         end
-        @searcher.find @request
-    end
 
-    it "should call process() on the first found entry" do
-        @connection.expects(:search).yields("myresult")
-        @searcher.expects(:process).with("myresult")
-        @searcher.find @request
-    end
-
-    it "should reconnect and retry the search if there is a failure" do
-        run = false
-        @connection.stubs(:search).with do |*args|
-            if run
+        it "should use the results of the ldapbase method as the ldap search base" do
+            @searcher.stubs(:search_base).returns("mybase")
+            @connection.expects(:search).with do |*args|
+                args[0].should == "mybase"
                 true
-            else
-                run = true
-                raise "failed"
             end
-        end.yields("myresult")
-        @searcher.expects(:process).with("myresult")
-
-        @searcher.find @request
-    end
-
-    it "should not reconnect on failure more than once" do
-        count = 0
-        @connection.stubs(:search).with do |*args|
-            count += 1
-            raise ArgumentError, "yay"
+            @searcher.find @request
         end
-        proc { @searcher.find(@request) }.should raise_error(Puppet::Error)
-        count.should == 2
-    end
-
-    it "should return true if an entry is found" do
-        @connection.expects(:search).yields("result")
-        @searcher.ldapsearch("whatever") { |r| }.should be_true
-    end
-end
 
-describe Puppet::Indirector::Ldap, " when connecting to ldap" do
-    confine "LDAP is not available" => Puppet.features.ldap?
-    confine "No LDAP test data for networks other than Luke's" => Facter.value(:domain) == "madstop.com"
-
-    it "should only create the ldap connection when asked for it the first time"
+        it "should default to the value of the :search_base setting as the result of the ldapbase method" do
+            Puppet.expects(:[]).with(:ldapbase).returns("myldapbase")
+            searcher = @ldap_class.new
+            searcher.search_base.should == "myldapbase"
+        end
 
-    it "should throw an exception if it cannot connect to LDAP"
+        it "should use the results of the :search_attributes method as the list of attributes to return" do
+            @searcher.stubs(:search_attributes).returns(:myattrs)
+            @connection.expects(:search).with do |*args|
+                args[3].should == :myattrs
+                true
+            end
+            @searcher.find @request
+        end
 
-    it "should use SSL when the :ldapssl setting is true"
+        it "should use depth 2 when searching" do
+            @connection.expects(:search).with do |*args|
+                args[1].should == 2
+                true
+            end
+            @searcher.find @request
+        end
 
-    it "should connect to the server specified in the :ldapserver setting"
+        it "should call process() on the first found entry" do
+            @connection.expects(:search).yields("myresult")
+            @searcher.expects(:process).with("myresult")
+            @searcher.find @request
+        end
 
-    it "should use the port specified in the :ldapport setting"
+        it "should reconnect and retry the search if there is a failure" do
+            run = false
+            @connection.stubs(:search).with do |*args|
+                if run
+                    true
+                else
+                    run = true
+                    raise "failed"
+                end
+            end.yields("myresult")
+            @searcher.expects(:process).with("myresult")
+
+            @searcher.find @request
+        end
 
-    it "should use protocol version 3"
+        it "should not reconnect on failure more than once" do
+            count = 0
+            @connection.stubs(:search).with do |*args|
+                count += 1
+                raise ArgumentError, "yay"
+            end
+            proc { @searcher.find(@request) }.should raise_error(Puppet::Error)
+            count.should == 2
+        end
 
-    it "should follow referrals"
+        it "should return true if an entry is found" do
+            @connection.expects(:search).yields("result")
+            @searcher.ldapsearch("whatever") { |r| }.should be_true
+        end
+    end
 
-    it "should use the user specified in the :ldapuser setting"
+    describe "when connecting to ldap" do
+        confine "LDAP is not available" => Puppet.features.ldap?
 
-    it "should use the password specified in the :ldappassord setting"
+        it "should only create the ldap connection when asked for it the first time" do
+            @searcher.connection.should equal(@searcher.connection)
+        end
 
-    it "should have an ldap method that returns an LDAP connection object"
+        it "should create and start a Util::Ldap::Connection instance" do
+            conn = mock 'connection', :connection => "myconn", :start => nil
+            Puppet::Util::Ldap::Connection.expects(:instance).returns conn
 
-    it "should fail when LDAP support is missing"
-end
+            @searcher.connection.should == "myconn"
+        end
+    end
 
-describe Puppet::Indirector::Ldap, " when reconnecting to ldap" do
-    confine "Not running on culain as root" => (Puppet::Util::SUIDManager.uid == 0 and Facter.value("hostname") == "culain")
+    describe "when reconnecting to ldap" do
+        confine "Not running on culain as root" => (Puppet::Util::SUIDManager.uid == 0 and Facter.value("hostname") == "culain")
 
-    it "should reconnect to ldap when connections are lost"
+        it "should reconnect to ldap when connections are lost"
+    end
 end
-- 
cgit 


From c1e010fb7dfe4666b3db3d958627f70f7325cf85 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 1 Jul 2008 22:20:26 -0500
Subject: 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.
---
 lib/puppet/indirector/node/ldap.rb       |  4 ++--
 spec/integration/indirector/node/ldap.rb | 22 ++++++++++++++++++++++
 spec/unit/indirector/node/ldap.rb        | 16 ++++++++++------
 3 files changed, 34 insertions(+), 8 deletions(-)
 create mode 100755 spec/integration/indirector/node/ldap.rb

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
-- 
cgit 


From d3a81255245eec19ac21902ae3b877e00e620628 Mon Sep 17 00:00:00 2001
From: Luke Kanies <luke@madstop.com>
Date: Tue, 1 Jul 2008 22:20:55 -0500
Subject: Fixed #1006 - puppetrun --class works again.  I added the class
 membership testing to the Ldap node terminus, and added tests,

---
 CHANGELOG     |  4 ++++
 bin/puppetrun | 48 +++++-------------------------------------------
 2 files changed, 9 insertions(+), 43 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index ae490cf8f..bdf722be5 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,4 +1,8 @@
 0.24.?
+    Fixed #1006 - puppetrun --class works again.  I added the class
+    membership testing to the Ldap node terminus, and added tests,
+    so it shouldn't break again.
+
     Fixed #1114 - Facts in plugin directories should now be autoloaded,
     as long as you're using Facter 1.5.
 
diff --git a/bin/puppetrun b/bin/puppetrun
index d0823b9c5..f1e30245b 100755
--- a/bin/puppetrun
+++ b/bin/puppetrun
@@ -139,51 +139,12 @@ begin
 rescue LoadError
     $stderr.puts "Failed to load ruby LDAP library. LDAP functionality will not be available"
 end
+
 require 'puppet'
 require 'puppet/network/client'
+require 'puppet/util/ldap/connection'
 require 'getoptlong'
 
-
-# Look up all nodes matching a given class in LDAP.
-def ldapnodes(klass, fqdn = true)
-    unless defined? @ldap
-        setupldap()
-    end
-
-    hosts = []
-
-    filter = nil
-    if klass == :all
-        filter = "objectclass=puppetclient"
-    else
-        filter = "puppetclass=#{klass}"
-    end
-    @ldap.search(Puppet[:ldapbase], 2, filter, "cn") do |entry|
-        # Skip the default host entry
-        if entry.dn =~ /cn=default,/
-            $stderr.puts "Skipping default host entry"
-            next
-        end
-
-        if fqdn
-            hosts << entry.dn.sub("cn=",'').sub(/ou=hosts,/i, '').gsub(",dc=",".")
-        else
-            hosts << entry.get_values("cn")[0]
-        end
-    end
-
-    return hosts
-end
-
-def setupldap
-    begin
-        @ldap = Puppet::Parser::Interpreter.ldap()
-    rescue => detail
-        $stderr.puts "Could not connect to LDAP: %s" % detail
-        exit(34)
-    end
-end
-
 flags = [
     [ "--all",      "-a",	        GetoptLong::NO_ARGUMENT ],
     [ "--tag",      "-t",           GetoptLong::REQUIRED_ARGUMENT ],
@@ -278,11 +239,12 @@ Puppet.parse_config
 
 if Puppet[:node_terminus] = "ldap"
     if options[:all]
-        hosts = ldapnodes(:all, options[:fqdn])
+        hosts = Puppet::Node.search("whatever").collect { |node| node.name }
         puts "all: %s" % hosts.join(", ")
     else
+        hosts = []
         classes.each do |klass|
-            list = ldapnodes(klass, options[:fqdn])
+            list = Puppet::Node.search("whatever", :class => klass).collect { |node| node.name }
             puts "%s: %s" % [klass, list.join(", ")]
 
             hosts += list
-- 
cgit