From 3e13bd59689a27a393c847bdbed3ac38765d79e9 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 20 May 2008 10:13:33 -0500 Subject: Intermediate commit so I can move on to other things. --- lib/puppet/provider/group/ldap.rb | 2 +- lib/puppet/provider/user/ldap.rb | 2 +- spec/unit/provider/user/ldap.rb | 10 ++++++++++ 3 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/puppet/provider/group/ldap.rb b/lib/puppet/provider/group/ldap.rb index 632358ff1..5af400a4e 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 diff --git a/lib/puppet/provider/user/ldap.rb b/lib/puppet/provider/user/ldap.rb index ba91a871e..d670ad435 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, diff --git a/spec/unit/provider/user/ldap.rb b/spec/unit/provider/user/ldap.rb index c4731cbbb..eb13d8bbd 100755 --- a/spec/unit/provider/user/ldap.rb +++ b/spec/unit/provider/user/ldap.rb @@ -24,6 +24,16 @@ describe provider_class do provider_class.manager.rdn.should == :uid end + it "should be unsuitable if ldap is unavailable" do + Puppet.features.expects(:ldap?).returns false + provider_class.should_not be_suitable + end + + it "should be suitable if ldap is available" do + Puppet.features.expects(:ldap?).returns true + provider_class.should be_suitable + end + {:name => "uid", :password => "userPassword", :comment => "cn", -- cgit From 419f2443c40116623b5c82f03eafcc85deeabcad Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 20 May 2008 23:59:04 -0500 Subject: Adding support for settings within the existing Facter provider confines. This renames the 'facter' confine to 'variable', and it prefers settings to facts. There shouldn't really be any overlap, so it shouldn't be a problem. --- CHANGELOG | 2 + lib/puppet/provider/confine/facter.rb | 37 ----------- lib/puppet/provider/confine/variable.rb | 42 ++++++++++++ lib/puppet/provider/confine_collection.rb | 4 +- lib/puppet/reference/providers.rb | 8 ++- spec/unit/provider/confine.rb | 4 +- spec/unit/provider/confine/facter.rb | 86 ------------------------- spec/unit/provider/confine/variable.rb | 102 ++++++++++++++++++++++++++++++ spec/unit/provider/confine_collection.rb | 10 +-- 9 files changed, 161 insertions(+), 134 deletions(-) delete mode 100644 lib/puppet/provider/confine/facter.rb create mode 100644 lib/puppet/provider/confine/variable.rb delete mode 100755 spec/unit/provider/confine/facter.rb create mode 100755 spec/unit/provider/confine/variable.rb diff --git a/CHANGELOG b/CHANGELOG index b0fbf7e4c..efbdf3737 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,5 @@ + Adding support for settings within the existing Facter provider confines. + 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/facter.rb deleted file mode 100644 index 9bb66c058..000000000 --- a/lib/puppet/provider/confine/facter.rb +++ /dev/null @@ -1,37 +0,0 @@ -require 'puppet/provider/confine' - -class Puppet::Provider::Confine::Facter < 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 - - # Retrieve the value from facter - def facter_value - unless defined?(@facter_value) and @facter_value - @facter_value = ::Facter.value(@fact).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(",")] - end - - def pass?(value) - facter_value == value.to_s.downcase - end - - def reset - # Reset the cache. We want to cache it during a given - # run, but across runs. - @facter_value = nil - end -end diff --git a/lib/puppet/provider/confine/variable.rb b/lib/puppet/provider/confine/variable.rb new file mode 100644 index 000000000..84d17367a --- /dev/null +++ b/lib/puppet/provider/confine/variable.rb @@ -0,0 +1,42 @@ +require '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 :name + + # Retrieve the value from facter + def facter_value + unless defined?(@facter_value) and @facter_value + @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.name, values.join(",")] + end + + def pass?(value) + test_value.downcase.to_s == value.to_s.downcase + end + + def reset + # Reset the cache. We want to cache it during a given + # 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/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/facter.rb deleted file mode 100755 index 560263257..000000000 --- a/spec/unit/provider/confine/facter.rb +++ /dev/null @@ -1,86 +0,0 @@ -#!/usr/bin/env ruby - -require File.dirname(__FILE__) + '/../../../spec_helper' - -require 'puppet/provider/confine/facter' - -describe Puppet::Provider::Confine::Facter::Facter do - it "should be named :facter" do - Puppet::Provider::Confine::Facter.name.should == :facter - end - - it "should require a value" do - lambda { Puppet::Provider::Confine::Facter.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) - end - - it "should have an accessor for its fact" do - Puppet::Provider::Confine::Facter.new(:bar).should respond_to(:fact) - end - - describe "when testing values" do - before { @confine = Puppet::Provider::Confine::Facter.new("foo") } - it "should use the 'pass?' method to test validity" do - @confine.expects(:pass?).with("foo") - @confine.valid? - end - - it "should return true if the value matches the facter value" do - Facter.expects(: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.pass?("foo").should be_false - end - - it "should be case insensitive" do - Facter.expects(: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.pass?(:foo).should be_true - end - - it "should cache the fact during testing" do - Facter.expects(:value).once.returns("FOO") - - @confine.pass?(:foo) - @confine.pass?(:foo) - end - - it "should produce a message that the fact value is not correct" do - @confine = Puppet::Provider::Confine::Facter.new(%w{bar bee}) - message = @confine.message("value") - message.should be_include("facter") - message.should be_include("bar,bee") - end - end - - describe "when summarizing multiple instances" do - it "should return a hash of failing variables and their values" do - c1 = stub '1', :valid? => false, :values => %w{one}, :fact => "uno" - 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}} - 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}} - end - end -end diff --git a/spec/unit/provider/confine/variable.rb b/spec/unit/provider/confine/variable.rb new file mode 100755 index 000000000..093301bdc --- /dev/null +++ b/spec/unit/provider/confine/variable.rb @@ -0,0 +1,102 @@ +#!/usr/bin/env ruby + +require File.dirname(__FILE__) + '/../../../spec_helper' + +require 'puppet/provider/confine/variable' + +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::Variable.new() }.should raise_error(ArgumentError) + end + + it "should always convert values to an array" do + Puppet::Provider::Confine::Variable.new("/some/file").values.should be_instance_of(Array) + end + + 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 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 + @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 + @confine.expects(:test_value).returns "fee" + + @confine.pass?("foo").should be_false + end + + it "should be case insensitive" do + @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 + @confine.expects(:test_value).returns "FOO" + + @confine.pass?(:foo).should be_true + end + + it "should cache the facter value during testing" do + Facter.expects(:value).once.returns("FOO") + + @confine.pass?(:foo) + @confine.pass?(:foo) + end + + it "should produce a message that the fact value is not correct" do + @confine = Puppet::Provider::Confine::Variable.new(%w{bar bee}) + message = @confine.message("value") + message.should be_include("facter") + message.should be_include("bar,bee") + end + end + + describe "when summarizing multiple instances" do + it "should return a hash of failing variables and their values" do + c1 = stub '1', :valid? => false, :values => %w{one}, :fact => "uno" + c2 = stub '2', :valid? => true, :values => %w{two}, :fact => "dos" + c3 = stub '3', :valid? => false, :values => %w{three}, :fact => "tres" + + 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::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 -- cgit From 4434072c7f51e4720b40aaea0637cb94dc6aefe5 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Wed, 21 May 2008 00:30:24 -0500 Subject: The ldap user/group providers now work when no users/groups are in ldap yet. Previously, they failed if you tried to get them to autogenerate an id, because they assumed that a result would be returned. --- lib/puppet/provider/group/ldap.rb | 14 ++++++++------ lib/puppet/provider/user/ldap.rb | 14 ++++++++------ spec/unit/provider/group/ldap.rb | 22 ++++++++++++++++++---- spec/unit/provider/user/ldap.rb | 31 ++++++++++++++++++------------- 4 files changed, 52 insertions(+), 29 deletions(-) diff --git a/lib/puppet/provider/group/ldap.rb b/lib/puppet/provider/group/ldap.rb index 5af400a4e..a4870fc68 100644 --- a/lib/puppet/provider/group/ldap.rb +++ b/lib/puppet/provider/group/ldap.rb @@ -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 d670ad435..0d149ac9a 100644 --- a/lib/puppet/provider/user/ldap.rb +++ b/lib/puppet/provider/user/ldap.rb @@ -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/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 eb13d8bbd..90fc7423f 100755 --- a/spec/unit/provider/user/ldap.rb +++ b/spec/unit/provider/user/ldap.rb @@ -24,16 +24,6 @@ describe provider_class do provider_class.manager.rdn.should == :uid end - it "should be unsuitable if ldap is unavailable" do - Puppet.features.expects(:ldap?).returns false - provider_class.should_not be_suitable - end - - it "should be suitable if ldap is available" do - Puppet.features.expects(:ldap?).returns true - provider_class.should be_suitable - end - {:name => "uid", :password => "userPassword", :comment => "cn", @@ -69,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} @@ -79,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 -- cgit