Merge branch 'ldap_fixes' into 0.24.x

Conflicts:

	CHANGELOG

11 files changed, 120 insertions, 60 deletions

diff --git a/CHANGELOG b/CHANGELOG
index 5b5148ff4..4fd0a3297 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,8 +1,10 @@
+    Adding support for settings within the existing Facter provider confines.
+
     Moving all confine code out of the Provider class, and fixing #1197. 
     Created a Confiner module for the Provider class methods, enhanced 
     the interface between it and the Confine class to make sure binary 
     paths are searched for fresh each time.
-    
+
     Modified the 'factpath' setting to automatically configure
     Facter to load facts there if a new enough version of
     Facter is used.
diff --git a/lib/puppet/provider/confine/facter.rb b/lib/puppet/provider/confine/variable.rb
index 9bb66c058..84d17367a 100644
--- a/lib/puppet/provider/confine/facter.rb
+++ b/lib/puppet/provider/confine/variable.rb
@@ -1,32 +1,27 @@
 require 'puppet/provider/confine'
 
-class Puppet::Provider::Confine::Facter < Puppet::Provider::Confine
+class Puppet::Provider::Confine::Variable < Puppet::Provider::Confine
     def self.summarize(confines)
         result = Hash.new { |hash, key| hash[key] = [] }
         confines.inject(result) { |total, confine| total[confine.fact] += confine.values unless confine.valid?; total }
     end
 
-    attr_accessor :fact
-
-    # Are we a facter comparison?
-    def facter?
-        defined?(@facter)
-    end
+    attr_accessor :name
 
     # Retrieve the value from facter
     def facter_value
         unless defined?(@facter_value) and @facter_value
-            @facter_value = ::Facter.value(@fact).to_s.downcase
+            @facter_value = ::Facter.value(name).to_s.downcase
         end
         @facter_value
     end
 
     def message(value)
-        "facter value '%s' for '%s' not in required list '%s'" % [value, self.fact, values.join(",")]
+        "facter value '%s' for '%s' not in required list '%s'" % [value, self.name, values.join(",")]
     end
 
     def pass?(value)
-        facter_value == value.to_s.downcase
+        test_value.downcase.to_s == value.to_s.downcase
     end
 
     def reset
@@ -34,4 +29,14 @@ class Puppet::Provider::Confine::Facter < Puppet::Provider::Confine
         # run, but across runs.
         @facter_value = nil
     end
+
+    private
+
+    def setting?
+        Puppet.settings.valid?(name)
+    end
+
+    def test_value
+        setting? ? Puppet.settings[name] : facter_value 
+    end
 end
diff --git a/lib/puppet/provider/confine_collection.rb b/lib/puppet/provider/confine_collection.rb
index 0c80086c9..35f461acb 100644
--- a/lib/puppet/provider/confine_collection.rb
+++ b/lib/puppet/provider/confine_collection.rb
@@ -15,8 +15,8 @@ class Puppet::Provider::ConfineCollection
                 @confines << klass.new(values)
                 @confines[-1].for_binary = true if for_binary
             else
-                confine = Puppet::Provider::Confine.test(:facter).new(values)
-                confine.fact = test
+                confine = Puppet::Provider::Confine.test(:variable).new(values)
+                confine.name = test
                 @confines << confine
             end
         end
diff --git a/lib/puppet/provider/group/ldap.rb b/lib/puppet/provider/group/ldap.rb
index 632358ff1..a4870fc68 100644
--- a/lib/puppet/provider/group/ldap.rb
+++ b/lib/puppet/provider/group/ldap.rb
@@ -12,7 +12,7 @@ Puppet::Type.type(:group).provide :ldap, :parent => Puppet::Provider::Ldap do
         as it iterates across all existing groups to pick the appropriate next
         one."
 
-    confine :true => Puppet.features.ldap?
+    confine :true => Puppet.features.ldap?, :false => (Puppet[:ldapuser] == "")
 
     # We're mapping 'members' here because we want to make it
     # easy for the ldap user provider to manage groups.  This
@@ -23,12 +23,14 @@ Puppet::Type.type(:group).provide :ldap, :parent => Puppet::Provider::Ldap do
     # Find the next gid after the current largest gid.
     provider = self
     manager.generates(:gidNumber).with do 
-        largest = 0
-        provider.manager.search.each do |hash|
-            next unless value = hash[:gid]
-            num = value[0].to_i
-            if num > largest
-                largest = num
+        largest = 500
+        if existing = provider.manager.search
+                existing.each do |hash|
+                next unless value = hash[:gid]
+                num = value[0].to_i
+                if num > largest
+                    largest = num
+                end
             end
         end
         largest + 1
diff --git a/lib/puppet/provider/user/ldap.rb b/lib/puppet/provider/user/ldap.rb
index ba91a871e..0d149ac9a 100644
--- a/lib/puppet/provider/user/ldap.rb
+++ b/lib/puppet/provider/user/ldap.rb
@@ -12,7 +12,7 @@ Puppet::Type.type(:user).provide :ldap, :parent => Puppet::Provider::Ldap do
         as it iterates across all existing users to pick the appropriate next
         one."
 
-    confine :true => Puppet.features.ldap?
+    confine :feature => :ldap, :false => (Puppet[:ldapuser] == "")
 
     manages(:posixAccount, :person).at("ou=People").named_by(:uid).and.maps :name => :uid,
         :password => :userPassword,
@@ -32,12 +32,14 @@ Puppet::Type.type(:user).provide :ldap, :parent => Puppet::Provider::Ldap do
     # Find the next uid after the current largest uid.
     provider = self
     manager.generates(:uidNumber).with do 
-        largest = 0
-        provider.manager.search.each do |hash|
-            next unless value = hash[:uid]
-            num = value[0].to_i
-            if num > largest
-                largest = num
+        largest = 500
+        if existing = provider.manager.search
+            existing.each do |hash|
+                next unless value = hash[:uid]
+                num = value[0].to_i
+                if num > largest
+                    largest = num
+                end
             end
         end
         largest + 1
diff --git a/lib/puppet/reference/providers.rb b/lib/puppet/reference/providers.rb
index 610c7550d..8fd2dbadc 100644
--- a/lib/puppet/reference/providers.rb
+++ b/lib/puppet/reference/providers.rb
@@ -63,9 +63,13 @@ providers = Puppet::Util::Reference.newreference :providers, :title => "Provider
                     case test
                     when :exists:
                         details += "  - Missing files %s\n" % values.join(", ")
-                    when :facter:
+                    when :variable:
                         values.each do |name, facts|
-                            details += "  - Fact %s (currently %s) not in list %s\n" % [name, Facter.value(name).inspect, facts.join(", ")]
+                            if Puppet.settings.valid?(name)
+                                details += "  - Setting %s (currently %s) not in list %s\n" % [name, Puppet.settings.value(name).inspect, facts.join(", ")]
+                            else
+                                details += "  - Fact %s (currently %s) not in list %s\n" % [name, Facter.value(name).inspect, facts.join(", ")]
+                            end
                         end
                     when :true:
                         details += "  - Got %s true tests that should have been false\n" % values
diff --git a/spec/unit/provider/confine.rb b/spec/unit/provider/confine.rb
index 6a9214e26..867b6e6be 100755
--- a/spec/unit/provider/confine.rb
+++ b/spec/unit/provider/confine.rb
@@ -29,8 +29,8 @@ describe Puppet::Provider::Confine do
         Puppet::Provider::Confine.test(:exists).should be_instance_of(Class)
     end
 
-    it "should have a 'facter' test" do
-        Puppet::Provider::Confine.test(:facter).should be_instance_of(Class)
+    it "should have a 'variable' test" do
+        Puppet::Provider::Confine.test(:variable).should be_instance_of(Class)
     end
 
     describe "when testing all values" do
diff --git a/spec/unit/provider/confine/facter.rb b/spec/unit/provider/confine/variable.rb
index 560263257..093301bdc 100755
--- a/spec/unit/provider/confine/facter.rb
+++ b/spec/unit/provider/confine/variable.rb
@@ -2,57 +2,73 @@
 
 require File.dirname(__FILE__) + '/../../../spec_helper'
 
-require 'puppet/provider/confine/facter'
+require 'puppet/provider/confine/variable'
 
-describe Puppet::Provider::Confine::Facter::Facter do
-    it "should be named :facter" do
-        Puppet::Provider::Confine::Facter.name.should == :facter
+describe Puppet::Provider::Confine::Variable do
+    it "should be named :variable" do
+        Puppet::Provider::Confine::Variable.name.should == :variable
     end
 
     it "should require a value" do
-        lambda { Puppet::Provider::Confine::Facter.new() }.should raise_error(ArgumentError)
+        lambda { Puppet::Provider::Confine::Variable.new() }.should raise_error(ArgumentError)
     end
 
     it "should always convert values to an array" do
-        Puppet::Provider::Confine::Facter.new("/some/file").values.should be_instance_of(Array)
+        Puppet::Provider::Confine::Variable.new("/some/file").values.should be_instance_of(Array)
     end
 
-    it "should have an accessor for its fact" do
-        Puppet::Provider::Confine::Facter.new(:bar).should respond_to(:fact)
+    it "should have an accessor for its name" do
+        Puppet::Provider::Confine::Variable.new(:bar).should respond_to(:name)
     end
 
     describe "when testing values" do
-        before { @confine = Puppet::Provider::Confine::Facter.new("foo") }
+        before do
+            @confine = Puppet::Provider::Confine::Variable.new("foo")
+            @confine.name = :myvar
+        end
+
         it "should use the 'pass?' method to test validity" do
             @confine.expects(:pass?).with("foo")
             @confine.valid?
         end
 
+        it "should use settings if the variable name is a valid setting" do
+            Puppet.settings.expects(:valid?).with(:myvar).returns true
+            Puppet.settings.expects(:value).with(:myvar).returns "foo"
+            @confine.pass?("foo")
+        end
+
+        it "should use Facter if the variable name is not a valid setting" do
+            Puppet.settings.expects(:valid?).with(:myvar).returns false
+            Facter.expects(:value).with(:myvar).returns "foo"
+            @confine.pass?("foo")
+        end
+
         it "should return true if the value matches the facter value" do
-            Facter.expects(:value).returns("foo")
+            @confine.expects(:test_value).returns "foo"
 
             @confine.pass?("foo").should be_true
         end
 
         it "should return false if the value does not match the facter value" do
-            Facter.expects(:value).returns("boo")
+            @confine.expects(:test_value).returns "fee"
 
             @confine.pass?("foo").should be_false
         end
 
         it "should be case insensitive" do
-            Facter.expects(:value).returns("FOO")
+            @confine.expects(:test_value).returns "FOO"
 
             @confine.pass?("foo").should be_true
         end
 
         it "should not care whether the value is a string or symbol" do
-            Facter.expects(:value).returns("FOO")
+            @confine.expects(:test_value).returns "FOO"
 
             @confine.pass?(:foo).should be_true
         end
 
-        it "should cache the fact during testing" do
+        it "should cache the facter value during testing" do
             Facter.expects(:value).once.returns("FOO")
 
             @confine.pass?(:foo)
@@ -60,7 +76,7 @@ describe Puppet::Provider::Confine::Facter::Facter do
         end
 
         it "should produce a message that the fact value is not correct" do
-            @confine = Puppet::Provider::Confine::Facter.new(%w{bar bee})
+            @confine = Puppet::Provider::Confine::Variable.new(%w{bar bee})
             message = @confine.message("value")
             message.should be_include("facter")
             message.should be_include("bar,bee")
@@ -73,14 +89,14 @@ describe Puppet::Provider::Confine::Facter::Facter do
             c2 = stub '2', :valid? => true,  :values => %w{two}, :fact => "dos"
             c3 = stub '3', :valid? => false, :values => %w{three}, :fact => "tres"
 
-            Puppet::Provider::Confine::Facter.summarize([c1, c2, c3]).should == {"uno" => %w{one}, "tres" => %w{three}}
+            Puppet::Provider::Confine::Variable.summarize([c1, c2, c3]).should == {"uno" => %w{one}, "tres" => %w{three}}
         end
 
         it "should combine the values of multiple confines with the same fact" do
             c1 = stub '1', :valid? => false, :values => %w{one}, :fact => "uno"
             c2 = stub '2', :valid? => false,  :values => %w{two}, :fact => "uno"
 
-            Puppet::Provider::Confine::Facter.summarize([c1, c2]).should == {"uno" => %w{one two}}
+            Puppet::Provider::Confine::Variable.summarize([c1, c2]).should == {"uno" => %w{one two}}
         end
     end
 end
diff --git a/spec/unit/provider/confine_collection.rb b/spec/unit/provider/confine_collection.rb
index da4b3fe72..1598b5f99 100755
--- a/spec/unit/provider/confine_collection.rb
+++ b/spec/unit/provider/confine_collection.rb
@@ -20,16 +20,16 @@ describe Puppet::Provider::ConfineCollection do
 
         describe "and the test cannot be found" do
             before do
-                @facter = mock 'facter_test'
+                @variable = mock 'variable_test'
 
                 Puppet::Provider::Confine.expects(:test).with(:foo).returns nil
-                Puppet::Provider::Confine.expects(:test).with(:facter).returns @facter
+                Puppet::Provider::Confine.expects(:test).with(:variable).returns @variable
             end
 
-            it "should create a Facter test with the provided values and set the fact to the test name" do
+            it "should create a Facter test with the provided values and set the name to the test name" do
                 confine = mock 'confine'
-                confine.expects(:fact=).with(:foo)
-                @facter.expects(:new).with(%w{my values}).returns confine
+                confine.expects(:name=).with(:foo)
+                @variable.expects(:new).with(%w{my values}).returns confine
                 Puppet::Provider::ConfineCollection.new.confine :foo => %w{my values}
             end
         end
diff --git a/spec/unit/provider/group/ldap.rb b/spec/unit/provider/group/ldap.rb
index 3f12d74e3..53d9e8bfc 100755
--- a/spec/unit/provider/group/ldap.rb
+++ b/spec/unit/provider/group/ldap.rb
@@ -45,8 +45,8 @@ describe provider_class do
 
         describe "with no gid specified" do
             it "should pick the first available GID after the largest existing GID" do
-                low = {:name=>["luke"], :gid=>["100"]}
-                high = {:name=>["testing"], :gid=>["140"]}
+                low = {:name=>["luke"], :gid=>["600"]}
+                high = {:name=>["testing"], :gid=>["640"]}
                 provider_class.manager.expects(:search).returns([low, high])
 
                 resource = stub 'resource', :should => %w{whatever}
@@ -55,12 +55,26 @@ describe provider_class do
                 instance = provider_class.new(:name => "luke", :ensure => :absent)
                 instance.stubs(:resource).returns resource
 
-                @connection.expects(:add).with { |dn, attrs| attrs["gidNumber"] == ["141"] }
+                @connection.expects(:add).with { |dn, attrs| attrs["gidNumber"] == ["641"] }
+
+                instance.create
+                instance.flush
+            end
+
+            it "should pick '501' as its GID if no groups are found" do
+                provider_class.manager.expects(:search).returns nil
+
+                resource = stub 'resource', :should => %w{whatever}
+                resource.stubs(:should).with(:gid).returns nil
+                resource.stubs(:should).with(:ensure).returns :present
+                instance = provider_class.new(:name => "luke", :ensure => :absent)
+                instance.stubs(:resource).returns resource
+
+                @connection.expects(:add).with { |dn, attrs| attrs["gidNumber"] == ["501"] }
 
                 instance.create
                 instance.flush
             end
         end
     end
-
 end
diff --git a/spec/unit/provider/user/ldap.rb b/spec/unit/provider/user/ldap.rb
index c4731cbbb..90fc7423f 100755
--- a/spec/unit/provider/user/ldap.rb
+++ b/spec/unit/provider/user/ldap.rb
@@ -59,8 +59,8 @@ describe provider_class do
 
         describe "with no uid specified" do
             it "should pick the first available UID after the largest existing UID" do
-                low = {:name=>["luke"], :shell=>:absent, :uid=>["100"], :home=>["/h"], :gid=>["1000"], :password=>["blah"], :comment=>["l k"]}
-                high = {:name=>["testing"], :shell=>:absent, :uid=>["140"], :home=>["/h"], :gid=>["1000"], :password=>["blah"], :comment=>["t u"]}
+                low = {:name=>["luke"], :shell=>:absent, :uid=>["600"], :home=>["/h"], :gid=>["1000"], :password=>["blah"], :comment=>["l k"]}
+                high = {:name=>["testing"], :shell=>:absent, :uid=>["640"], :home=>["/h"], :gid=>["1000"], :password=>["blah"], :comment=>["t u"]}
                 provider_class.manager.expects(:search).returns([low, high])
 
                 resource = stub 'resource', :should => %w{whatever}
@@ -69,7 +69,22 @@ describe provider_class do
                 instance = provider_class.new(:name => "luke", :ensure => :absent)
                 instance.stubs(:resource).returns resource
 
-                @connection.expects(:add).with { |dn, attrs| attrs["uidNumber"] == ["141"] }
+                @connection.expects(:add).with { |dn, attrs| attrs["uidNumber"] == ["641"] }
+
+                instance.create
+                instance.flush
+            end
+
+            it "should pick 501 of no users exist" do
+                provider_class.manager.expects(:search).returns nil
+
+                resource = stub 'resource', :should => %w{whatever}
+                resource.stubs(:should).with(:uid).returns nil
+                resource.stubs(:should).with(:ensure).returns :present
+                instance = provider_class.new(:name => "luke", :ensure => :absent)
+                instance.stubs(:resource).returns resource
+
+                @connection.expects(:add).with { |dn, attrs| attrs["uidNumber"] == ["501"] }
 
                 instance.create
                 instance.flush