diff options
| -rw-r--r-- | lib/puppet/statechange.rb | 6 | ||||
| -rw-r--r-- | lib/puppet/transaction.rb | 1 | ||||
| -rwxr-xr-x | lib/puppet/type/group.rb | 9 | ||||
| -rwxr-xr-x | lib/puppet/type/user.rb | 210 | ||||
| -rw-r--r-- | test/puppettest.rb | 34 | ||||
| -rwxr-xr-x | test/types/tc_group.rb | 35 | ||||
| -rwxr-xr-x | test/types/tc_user.rb | 335 |
7 files changed, 469 insertions, 161 deletions
diff --git a/lib/puppet/statechange.rb b/lib/puppet/statechange.rb index ac10dc301..af066a773 100644 --- a/lib/puppet/statechange.rb +++ b/lib/puppet/statechange.rb @@ -48,7 +48,7 @@ module Puppet if event.nil? #event = @state.parent.class.name.id2name + "_changed" # they didn't actually change anything - return + return nil elsif ! event.is_a?(Symbol) Puppet.warning "State '%s' returned invalid event '%s'; resetting to default" % [@state.class,event] @@ -111,6 +111,10 @@ module Puppet unless @state.insync? Puppet.notice "Rolling %s backward" % self return self.go + else + Puppet.debug "rollback is already in sync: %s vs. %s" % + [@state.is.inspect, @state.should.inspect] + return nil end #raise "Moving statechanges backward is currently unsupported" diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index 2e726df0b..f0ef8469d 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -115,6 +115,7 @@ class Transaction def rollback events = @changes.collect { |change| if change.is_a?(Puppet::StateChange) + # skip changes that were never actually run next unless change.run #change.transaction = self begin diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb index 38a56dc31..fc8b7555d 100755 --- a/lib/puppet/type/group.rb +++ b/lib/puppet/type/group.rb @@ -100,7 +100,12 @@ module Puppet :name ] - @doc = " " + @doc = "Manage groups. This type can only create groups. Group + membership must be managed on individual users." + + @paramdoc[:name] = "The group name. While naming limitations vary by + system, it is advisable to keep the name to the degenerate limitations, + which is a maximum of 8 characters beginning with a letter." @name = :group @namevar = :name @@ -109,7 +114,7 @@ module Puppet begin @groupinfo = Etc.getgrnam(self[:name]) rescue ArgumentError => detail - # leave groupinfo as nil + @groupinfo = nil end end diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb index cc7bbadaa..c4b31ed3e 100755 --- a/lib/puppet/type/user.rb +++ b/lib/puppet/type/user.rb @@ -7,43 +7,95 @@ require 'puppet/type/state' module Puppet class State class UserState < Puppet::State - @@userinfo = nil - class << self - attr_accessor :infomethod - - def getinfo(refresh = false) - if @@userinfo.nil? or refresh == true - begin - @@userinfo = Etc.getpwnam(@parent[:name]) - rescue ArgumentError => detail - @@userinfo = :notfound - end + attr_accessor :flag + def infomethod + if defined? @infomethod and @infomethod + return @infomethod + else + return @name + end + end + end + + def create + obj = @parent.getinfo + + cmd = nil + event = nil + if @should == :notfound + # we need to remove the object... + if obj.nil? + # the user already doesn't exist + return nil end - @@userinfo + cmd = ["userdel", @parent.name] + type = "delete" + else + unless obj.nil? + raise Puppet::DevError, + "Got told to create a user that already exists" + end + # we're creating the user + + # i can just tell i'm going to regret this + # why doesn't POSIX include interfaces for adding users + # and groups? it's stupid + cmd = ["useradd"] + @parent.eachstate { |state| + # the value needs to be quoted, mostly because -c might + # have spaces in it + cmd << state.class.flag << "'%s'" % state.should + } + cmd << @parent.name + type = "create" + end + + output = %x{#{cmd.join(" ")} 2>&1} + + unless $? == 0 + raise Puppet::Error, "Could not %s group %s: %s" % + [type, @parent.name, output] end + + return "user_#{type}d".intern end def retrieve - info = self.class.getinfo(true) + if info = @parent.getinfo(true) + @is = info.send(self.class.infomethod) + else + @is = :notfound + end + end - method = self.class.infomethod || self.class.name + def sync + obj = @parent.getinfo - unless method - raise Puppet::DevError, - "Could not retrieve info method for state %s" % self.class.name + # if the user either does not or should not exist... + # yes, it's a badly named method + if obj.nil? or @should == :notfound + return self.create end - unless info.respond_to?(method) - raise Puppet::DevError, "UserInfo object does not respond to %s" % - method + # there's a possibility that we created the user in this session + # so verify that we're actually out of sync + if self.insync? + return nil end + cmd = [ + "usermod", self.class.flag, "'%s'" % @should, @parent.name + ].join(" ") - @is = info.send(method) - end + output = %x{#{cmd} 2>&1} - def sync + unless $? == 0 + raise Puppet::Error, "Could not modify %s on user %s: %s" % + [self.class.name, @parent.name, output] + end + + return :user_modified end end @@ -53,11 +105,13 @@ module Puppet automatically, which will likely result in the same user having different IDs on different systems, which is not recommended." @name = :uid + @flag = "-u" end class UserGID < UserState @doc = "The user's primary group. Can be specified numerically or by name." @name = :gid + @flag = "-g" def should=(gid) method = :getgrgid @@ -79,88 +133,54 @@ module Puppet @should = ginfo.gid end - - def sync - return :executed_command - end end class UserComment < UserState - @doc = "The expected return code. An error will be returned if the - executed command returns something else." + @doc = "A description of the user. Generally is a user's full name." @name = :comment - - def retrieve - end - - def sync - return :executed_command - end + @infomethod = :gecos + @flag = "-c" end class UserHome < UserState - @doc = "The expected return code. An error will be returned if the - executed command returns something else." + @doc = "The home directory of the user. The directory must be created + separately and is not currently checked for existence." @name = :home - - def retrieve - end - - def sync - return :executed_command - end + @infomethod = :dir + @flag = "-d" end class UserShell < UserState - @doc = "The expected return code. An error will be returned if the - executed command returns something else." + @doc = "The user's login shell. The shell must exist and be + executable." @name = :shell - - def retrieve - end - - def sync - return :executed_command - end + @flag = "-s" end + # these three states are all implemented differently on each platform, so i'm + # disabling them for now + + # FIXME Puppet::State::UserLocked is currently non-functional class UserLocked < UserState @doc = "The expected return code. An error will be returned if the executed command returns something else." @name = :locked - - def retrieve - end - - def sync - return :executed_command - end end + # FIXME Puppet::State::UserExpire is currently non-functional class UserExpire < UserState @doc = "The expected return code. An error will be returned if the executed command returns something else." @name = :expire - - def retrieve - end - - def sync - return :executed_command - end + @flag = "-e" end + # FIXME Puppet::State::UserInactive is currently non-functional class UserInactive < UserState @doc = "The expected return code. An error will be returned if the executed command returns something else." @name = :inactive - - def retrieve - end - - def sync - return :executed_command - end + @flag = "-f" end end @@ -172,25 +192,49 @@ module Puppet Puppet::State::UserGID, Puppet::State::UserComment, Puppet::State::UserHome, - Puppet::State::UserShell, - Puppet::State::UserLocked, - Puppet::State::UserExpire, - Puppet::State::UserInactive + Puppet::State::UserShell ] @parameters = [ :name ] - @doc = " - " + @paramdoc[:name] = "User name. While limitations are determined for + each operating system, it is generally a good idea to keep to the + degenerate 8 characters, beginning with a letter." + + @doc = "Manage users. Currently can create and modify users, but cannot + delete them. Theoretically all of the parameters are optional, + but if no parameters are specified the comment will be set to the + user name in order to make the internals work out correctly." @name = :user @namevar = :name + def getinfo(refresh = false) + if @userinfo.nil? or refresh == true + begin + @userinfo = Etc.getpwnam(self[:name]) + rescue ArgumentError => detail + @userinfo = nil + end + end + + @userinfo + end + + def initialize(hash) + @userinfo = nil + super + + if @states.empty? + self[:comment] = self[:name] + end + end + def retrieve - info = Puppet::State::UserState.getinfo + info = self.getinfo(true) - if info == :notfound + if info.nil? # the user does not exist @states.each { |name, state| state.is = :notfound diff --git a/test/puppettest.rb b/test/puppettest.rb index 642c8275f..1543434e9 100644 --- a/test/puppettest.rb +++ b/test/puppettest.rb @@ -27,6 +27,40 @@ class TestPuppet < Test::Unit::TestCase Puppet::Type.allclear end + def assert_rollback_events(trans, events, msg) + run_events(:rollback, trans, events, msg) + end + + def assert_events(comp, events, msg) + trans = nil + assert_nothing_raised("Component %s failed" % [msg]) { + trans = comp.evaluate + } + + run_events(:evaluate, trans, events, msg) + end + + def run_events(type, trans, events, msg) + case type + when :evaluate, :rollback: # things are hunky-dory + else + raise Puppet::DevError, "Incorrect run_events type" + end + + method = type + + newevents = nil + assert_nothing_raised("Transaction %s %s failed" % [type, msg]) { + newevents = trans.send(method).reject { |e| e.nil? }.collect { |e| + e.event + } + } + + assert_equal(events, newevents, "Incorrect %s %s events" % [type, msg]) + + return trans + end + def test_nothing end end diff --git a/test/types/tc_group.rb b/test/types/tc_group.rb index 7f3c1d01f..2a23535d4 100755 --- a/test/types/tc_group.rb +++ b/test/types/tc_group.rb @@ -14,6 +14,19 @@ require 'test/unit' class TestGroup < TestPuppet def setup Puppet[:loglevel] = :debug if __FILE__ == $0 + @@tmpgroups = [] + super + end + + def teardown + @@tmpgroups.each { |group| + begin + obj = Etc.getgrnam(group) + system("groupdel %s" % group) + rescue ArgumentError => detail + # no such group, so we're fine + end + } super end @@ -67,6 +80,10 @@ class TestGroup < TestPuppet gobj = nil comp = nil name = "pptestgr" + + assert_raise(ArgumentError) { + obj = Etc.getgrnam(name) + } assert_nothing_raised { gobj = Puppet::Type::Group.new( :name => name @@ -75,23 +92,19 @@ class TestGroup < TestPuppet comp = newcomp("groupmaker %s" % name, gobj) } - trans = nil - assert_nothing_raised { - trans = comp.evaluate - } + trans = assert_events(comp, [:group_created], "group") + @@tmpgroups << name - events = nil + obj = nil assert_nothing_raised { - events = trans.evaluate.reject { |e| e.nil? }.collect { |e| e.event } + obj = Etc.getgrnam(name) } - assert_equal([:group_created], events, "Incorrect group events") + assert_rollback_events(trans, [:group_deleted], "group") - assert_nothing_raised { - events = trans.rollback.reject { |e| e.nil? }.collect { |e| e.event } + assert_raise(ArgumentError) { + obj = Etc.getgrnam(name) } - - assert_equal([:group_deleted], events, "Incorrect deletion group events") end else $stderr.puts "Not running as root; skipping group creation tests." diff --git a/test/types/tc_user.rb b/test/types/tc_user.rb index 3522539d5..a70e55ce6 100755 --- a/test/types/tc_user.rb +++ b/test/types/tc_user.rb @@ -6,93 +6,300 @@ end # $Id$ +require 'etc' require 'puppet/type' +require 'puppettest' require 'test/unit' -class TestType < Test::Unit::TestCase - def test_typemethods - assert_nothing_raised() { - Puppet::Type.buildstatehash +class TestUser < TestPuppet + def setup + @@tmpusers = [] + Puppet[:loglevel] = :debug if __FILE__ == $0 + super + end + + def teardown + @@tmpusers.each { |user| + begin + obj = Etc.getpwnam(user) + system("userdel %s" % user) + rescue ArgumentError => detail + # no such user, so we're fine + end } + super + end - Puppet::Type.eachtype { |type| - name = nil - assert_nothing_raised() { - name = type.name - } + def attrtest_comment(user) + old = user.is(:comment) + user[:comment] = "A different comment" - assert( - name - ) + comp = newcomp("commenttest", user) - assert_equal( - type, - Puppet::Type.type(name) - ) + trans = assert_events(comp, [:user_modified], "user") - assert( - type.namevar - ) + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) + } - assert_not_nil( - type.states - ) + assert_equal("A different comment", obj.gecos, "Comment was not changed") - assert_not_nil( - type.validstates - ) + assert_rollback_events(trans, [:user_modified], "user") - assert( - type.validparameter?(type.namevar) - ) + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) } + + assert_equal(old, obj.gecos, "Comment was not reverted") end - def test_stringvssymbols - file = nil - path = "/tmp/testfile" - assert_nothing_raised() { - system("rm -f %s" % path) - file = Puppet::Type::PFile.new( - :path => path, - :create => true, - :recurse => true, - :checksum => "md5" - ) + def attrtest_home(user) + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) } - assert_nothing_raised() { - file.retrieve + old = obj.dir + comp = newcomp("hometest", user) + + user[:home] = old + + trans = assert_events(comp, [], "user") + + user[:home] = "/tmp" + + trans = assert_events(comp, [:user_modified], "user") + + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) } - assert_nothing_raised() { - file.sync + + assert_equal("/tmp", obj.dir, "Home was not changed") + + assert_rollback_events(trans, [:user_modified], "user") + + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) } - Puppet::Type::PFile.clear - assert_nothing_raised() { - system("rm -f %s" % path) - file = Puppet::Type::PFile.new( - "path" => path, - "create" => true, - "recurse" => true, - "checksum" => "md5" - ) + + assert_equal(old, obj.dir, "Home was not reverted") + end + + def attrtest_shell(user) + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) } - assert_nothing_raised() { - file.retrieve + old = obj.shell + comp = newcomp("shelltest", user) + + user[:shell] = old + + trans = assert_events(comp, [], "user") + + newshell = %w{/bin/sh /bin/bash /sbin/sh /bin/ksh /bin/zsh /bin/csh /bin/tcsh + /usr/bin/sh /usr/bin/bash /usr/bin/ksh /usr/bin/zsh /usr/bin/csh + /usr/bin/tcsh}.find { |shell| + FileTest.exists?(shell) and shell != old } - assert_nothing_raised() { - file[:path] + + unless newshell + $stderr.puts "Cannot find alternate shell; skipping shell test" + return + end + + user[:shell] = newshell + + trans = assert_events(comp, [:user_modified], "user") + + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) + } + + assert_equal(newshell, obj.shell, "Shell was not changed") + + assert_rollback_events(trans, [:user_modified], "user") + + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) } - assert_nothing_raised() { - file["path"] + + assert_equal(old, obj.shell, "Shell was not reverted") + end + + def attrtest_gid(user) + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) + } + old = obj.gid + comp = newcomp("gidtest", user) + + user[:gid] = old + + trans = assert_events(comp, [], "user") + + newgid = %w{nogroup nobody staff users daemon}.find { |gid| + begin + group = Etc.getgrnam(gid) + rescue ArgumentError => detail + false + end + old != group.gid } - assert_nothing_raised() { - file[:recurse] + + unless newgid + $stderr.puts "Cannot find alternate group; skipping gid test" + return + end + + # first test by name + assert_nothing_raised("Failed to specify group by name") { + user[:gid] = newgid } - assert_nothing_raised() { - file["recurse"] + + trans = assert_events(comp, [:user_modified], "user") + + # then by id + newgid = Etc.getgrnam(newgid).gid + + assert_nothing_raised("Failed to specify group by id") { + user[:gid] = newgid } - assert_nothing_raised() { - file.sync + + assert_events(comp, [], "user") + + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) } + + assert_equal(newgid, obj.gid, "GID was not changed") + + assert_rollback_events(trans, [:user_modified], "user") + + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) + } + + assert_equal(old, obj.gid, "GID was not reverted") + end + + def attrtest_uid(user) + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) + } + old = obj.uid + comp = newcomp("uidtest", user) + + user[:uid] = old + + trans = assert_events(comp, [], "user") + + newuid = old + while true + newuid += 1 + + if newuid - old > 1000 + $stderr.puts "Could not find extra test UID" + return + end + begin + newuser = Etc.getpwuid(newuid) + rescue ArgumentError => detail + break + end + end + + assert_nothing_raised("Failed to change user id") { + user[:uid] = newuid + } + + trans = assert_events(comp, [:user_modified], "user") + + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) + } + + assert_equal(newuid, obj.uid, "UID was not changed") + + assert_rollback_events(trans, [:user_modified], "user") + + assert_nothing_raised { + obj = Etc.getpwnam(user[:name]) + } + + assert_equal(old, obj.uid, "UID was not reverted") + end + + def test_eachmethod + obj = Etc.getpwuid(Process.uid) + + assert(obj, "Could not retrieve test group object") + + Puppet::Type::User.validstates.each { |name, state| + assert_nothing_raised { + method = state.infomethod + assert(method, "State %s has no infomethod" % name) + assert(obj.respond_to?(method), "State %s has an invalid method %s" % + [name, method]) + } + } + end + + if Process.uid == 0 + def test_simpleuser + user = nil + name = "pptest" + + assert_raise(ArgumentError) { + Etc.getpwnam(name) + } + + assert_nothing_raised { + user = Puppet::Type::User.new( + :name => name, + :comment => "Puppet Testing User" + ) + } + + comp = newcomp("usercomp", user) + + trans = assert_events(comp, [:user_created], "user") + + @@tmpusers << name + + obj = nil + assert_nothing_raised { + obj = Etc.getpwnam(name) + } + + assert_equal("Puppet Testing User", obj.gecos, "Comment was not set") + + tests = Puppet::Type::User.validstates.collect { |name, state| + state.name + } + + user.retrieve + tests.each { |test| + if self.respond_to?("attrtest_%s" % test) + self.send("attrtest_%s" % test, user) + else + $stderr.puts "Not testing attr %s of user" % test + end + } + + assert_rollback_events(trans, [:user_deleted], "user") + + assert_raise(ArgumentError) { + Etc.getpwnam(user[:name]) + } + end + else + $stderr.puts "Not root; skipping user creation/modification tests" end end |