diff options
| -rw-r--r-- | lib/puppet/log.rb | 4 | ||||
| -rw-r--r-- | lib/puppet/type.rb | 46 | ||||
| -rwxr-xr-x | lib/puppet/type/pfile/uid.rb | 106 | ||||
| -rw-r--r-- | test/other/log.rb | 12 | ||||
| -rw-r--r-- | test/types/file.rb | 43 | ||||
| -rw-r--r-- | test/types/type.rb | 43 |
6 files changed, 152 insertions, 102 deletions
diff --git a/lib/puppet/log.rb b/lib/puppet/log.rb index 62fe98fdf..7c30e88b5 100644 --- a/lib/puppet/log.rb +++ b/lib/puppet/log.rb @@ -167,10 +167,10 @@ module Puppet # :nodoc: # cannot log a message with a '%' in it. So, we get rid # of them. if msg.source == "Puppet" - dest.send(msg.level, msg.to_s.gsub("%", '')) + dest.send(msg.level, msg.to_s.gsub("%", '%%')) else dest.send(msg.level, "(%s) %s" % - [msg.source.to_s.gsub("%", ""), msg.to_s.gsub("%", '')] + [msg.source.to_s.gsub("%", ""), msg.to_s.gsub("%", '%%')] ) end when File: diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index 8a08152f8..9d728c07c 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -808,36 +808,46 @@ class Type < Puppet::Element self.class[self.name] = self end - # merge new information with an existing object, checking for conflicts - # and such + # Merge new information with an existing object, checking for conflicts + # and such. This allows for two specifications of the same object and + # the same values, but it's pretty limited right now. The result of merging + # states is very different from the result of merging parameters or metaparams. def merge(hash) hash.each { |param, value| if param.is_a?(String) param = param.intern end + + # Of course names are the same, duh. next if param == :name or param == self.class.namevar unless value.is_a?(Array) value = [value] end - # This needs some way to retrieve the original values specified to - # 'should'. - if oldvals = self.should(param) - if oldvals.is_a?(Array) - # take the intersection - newvals = oldvals & value - if newvals.empty? - raise Puppet::Error, "No common values for %s on %s(%s)" % - [param, self.class.name, self.name] - else - self.debug "Reduced old values %s and new values %s to %s" % - [oldvals.inspect, value.inspect, newvals.inspect] - self.should = newvals - end - else - raise Puppet::Error, "Cannot override %s on %s(%s)" % + if oldvals = @states[param].shouldorig + unless oldvals.is_a?(Array) + oldvals = [oldvals] + end + # If the values are exactly the same, order and everything, + # then it's okay. + if oldvals == value + return true + end + # take the intersection + newvals = oldvals & value + if newvals.empty? + raise Puppet::Error, "No common values for %s on %s(%s)" % + [param, self.class.name, self.name] + elsif newvals.length > 1 + raise Puppet::Error, "Too many values for %s on %s(%s)" % [param, self.class.name, self.name] + else + self.debug "Reduced old values %s and new values %s to %s" % + [oldvals.inspect, value.inspect, newvals.inspect] + @states[param].should = newvals + #self.should = newvals + return true end else self[param] = value diff --git a/lib/puppet/type/pfile/uid.rb b/lib/puppet/type/pfile/uid.rb index b61fa8af7..299d92756 100755 --- a/lib/puppet/type/pfile/uid.rb +++ b/lib/puppet/type/pfile/uid.rb @@ -20,6 +20,40 @@ module Puppet end end + def name2id(value) + begin + user = Etc.getpwnam(value) + if user.uid == "" + return nil + end + return user.uid + rescue ArgumentError => detail + return nil + end + end + + # Determine if the user is valid, and if so, return the UID + def validuser?(value) + if value =~ /^\d+$/ + value = value.to_i + end + + if value.is_a?(Integer) + # verify the user is a valid user + if tmp = id2name(value) + return value + else + return false + end + else + if tmp = name2id(value) + return tmp + else + return false + end + end + end + # We want to print names, not numbers def is_to_s id2name(@is) || @is @@ -38,49 +72,17 @@ module Puppet self.is = stat.uid end - # If we're not root, we can check the values but we cannot change them + # If we're not root, we can check the values but we cannot change them. + # We can't really do any processing here, because users might + # not exist yet. FIXME There's still a bit of a problem here if + # the user's UID changes at run time, but we're just going to have + # to be okay with that for now, unfortunately. def shouldprocess(value) - if value.is_a?(Integer) - # verify the user is a valid user - begin - user = Etc.getpwuid(value) - if user.uid == "" - error = Puppet::Error.new( - "Could not retrieve uid for '%s'" % - @parent.name) - raise error - end - rescue ArgumentError => detail - raise Puppet::Error.new("User ID %s does not exist" % - value - ) - rescue => detail - raise Puppet::Error.new( - "Could not find user '%s': %s" % [value, detail]) - raise error - end + if tmp = self.validuser?(value) + return tmp else - begin - user = Etc.getpwnam(value) - if user.uid == "" - error = Puppet::Error.new( - "Could not retrieve uid for '%s'" % - @parent.name) - raise error - end - value = user.uid - rescue ArgumentError => detail - raise Puppet::Error.new("User %s does not exist" % - value - ) - rescue => detail - error = Puppet::Error.new( - "Could not find user '%s': %s" % [value, detail]) - raise error - end + return value end - - return value end def sync @@ -90,13 +92,25 @@ module Puppet #@parent.delete(self.name) @@notifieduid = true end - # there's a possibility that we never got retrieve() called - # e.g., if the file didn't exist - # thus, just delete ourselves now and don't do any work - #@parent.delete(self.name) return nil end + user = nil + unless user = self.validuser?(self.should) + tmp = self.should + unless defined? @@usermissing + @@usermissing = {} + end + + if @@usermissing.include?(tmp) + @@usermissing[tmp] += 1 + else + self.notice "user %s does not exist" % tmp + @@usermissing[tmp] = 1 + return nil + end + end + if @is == :notfound @parent.stat(true) self.retrieve @@ -111,10 +125,10 @@ module Puppet end begin - File.chown(self.should,nil,@parent[:path]) + File.chown(user, nil, @parent[:path]) rescue => detail raise Puppet::Error, "Failed to set owner to '%s': %s" % - [self.should,detail] + [user, detail] end return :inode_changed diff --git a/test/other/log.rb b/test/other/log.rb index f25b01149..0fd72887a 100644 --- a/test/other/log.rb +++ b/test/other/log.rb @@ -141,4 +141,16 @@ class TestLog < Test::Unit::TestCase assert_equal(log.tags, file.tags, "Tags were not equal") assert_equal(log.source, file.path, "Source was not set correctly") end + + # Verify that we can pass strings that match printf args + def test_percentlogs + Puppet[:logdest] = :syslog + + assert_nothing_raised { + Puppet::Log.new( + :level => :info, + :message => "A message with %s in it" + ) + } + end end diff --git a/test/types/file.rb b/test/types/file.rb index 4ff0705c0..b160ae1da 100644 --- a/test/types/file.rb +++ b/test/types/file.rb @@ -32,12 +32,12 @@ class TestFile < Test::Unit::TestCase end def setup + super begin initstorage rescue system("rm -rf %s" % Puppet[:checksumfile]) end - super end def teardown @@ -192,14 +192,15 @@ class TestFile < Test::Unit::TestCase assert(file.insync?()) } - fake.each { |uid, name| - assert_raise(Puppet::Error) { - file[:owner] = name - } - assert_raise(Puppet::Error) { - file[:owner] = uid - } - } + # We no longer raise an error here, because we check at run time + #fake.each { |uid, name| + # assert_raise(Puppet::Error) { + # file[:owner] = name + # } + # assert_raise(Puppet::Error) { + # file[:owner] = uid + # } + #} end def test_groupasroot @@ -518,30 +519,6 @@ class TestFile < Test::Unit::TestCase file.sync } end - - if Process.uid == 0 - def test_zfilewithpercentsign - file = nil - dir = tmpdir() - path = File.join(dir, "file%sname") - assert_nothing_raised { - file = Puppet::Type::PFile.create( - :path => path, - :create => true, - :owner => "nosuchuser", - :group => "root", - :mode => "755" - ) - } - - comp = newcomp("percent", file) - events = nil - assert_nothing_raised { - trans = comp.evaluate - events = trans.evaluate - } - end - end end # $Id$ diff --git a/test/types/type.rb b/test/types/type.rb index 4347ea0b0..55b809372 100644 --- a/test/types/type.rb +++ b/test/types/type.rb @@ -1,11 +1,9 @@ if __FILE__ == $0 $:.unshift '..' $:.unshift '../../lib' - $puppetbase = "../../../../language/trunk" + $puppetbase = "../.." end -# $Id$ - require 'puppet/type' require 'puppettest' require 'test/unit' @@ -111,4 +109,43 @@ class TestType < Test::Unit::TestCase assert_equal("testing", group.name, "Could not retrieve name") end + + # Verify that values get merged correctly + def test_mergestatevalues + file = tempfile() + + # Create the first version + assert_nothing_raised { + Puppet::Type::PFile.create( + :path => file, + :owner => ["root", "bin"] + ) + } + + # Make an identical statement + assert_nothing_raised { + Puppet::Type::PFile.create( + :path => file, + :owner => ["root", "bin"] + ) + } + + # Create a conflicting statement + assert_raise(Puppet::Error) { + Puppet::Type::PFile.create( + :path => file, + :owner => ["root", "bin", "adm"] + ) + } + + # And then make an intersection with one valid value + assert_nothing_raised { + Puppet::Type::PFile.create( + :path => file, + :owner => "root" + ) + } + end end + +# $Id$ |