From 4c998fe67d7e82c91d5fefd3c0239cb132e9a16d Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 30 Sep 2008 17:49:18 -0500 Subject: Fixing #1622 - The user type only looks up groups when necessary. Also added a bunch of tests to the user type, and refactored as necessary for this to work. Signed-off-by: Luke Kanies --- lib/puppet/type/user.rb | 71 ++++++++++--------------------------------------- 1 file changed, 14 insertions(+), 57 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb index 039bcb7cb..79cd5ff26 100755 --- a/lib/puppet/type/user.rb +++ b/lib/puppet/type/user.rb @@ -49,25 +49,6 @@ module Puppet return :absent end end - - # The default 'sync' method only selects among a list of registered - # values. - def sync -# if self.insync? -# self.info "already in sync" -# return nil - #else - #self.info "%s vs %s" % [self.is.inspect, self.should.inspect] -# end - unless self.class.values - self.devfail "No values defined for %s" % - self.class.name - end - - # Set ourselves to whatever our should value is. - self.set(self.should) - end - end newproperty(:uid) do @@ -95,50 +76,26 @@ module Puppet newproperty(:gid) do desc "The user's primary group. Can be specified numerically or by name." - - def found? - defined? @found and @found - end - - munge do |gid| - method = :getgrgid - case gid - when String - if gid =~ /^[-0-9]+$/ - gid = Integer(gid) - else - method = :getgrnam - end - when Symbol - unless gid == :auto or gid == :absent - self.devfail "Invalid GID %s" % gid - end - # these are treated specially by sync() - return gid - end - if group = Puppet::Util.gid(gid) - @found = true - return group + munge do |value| + if value.is_a?(String) and value =~ /^[-0-9]+$/ + Integer(value) else - @found = false - return gid + value end end - # *shudder* Make sure that we've looked up the group and gotten - # an ID for it. Yuck-o. - def should - unless defined? @should - return super - end - unless found? - @should = @should.each { |val| - next unless val - Puppet::Util.gid(val) - } + def sync + found = false + @should.each do |value| + if number = Puppet::Util.gid(value) + provider.gid = number + found = true + break + end end - super + + fail "Could not find group(s) %s" % @should.join(",") unless found end end -- cgit From 2480654aa07dbe6c50777417143a133b1cd94859 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 30 Sep 2008 17:50:31 -0500 Subject: The Netinfo and DirectoryService providers can now create user and group simultaneously. This required selectively using property#sync if a 'should' value is present, so that the user's gid property can do the conversion if necessary. Signed-off-by: Luke Kanies --- lib/puppet/provider/nameservice/directoryservice.rb | 15 ++++++++++++--- lib/puppet/provider/nameservice/netinfo.rb | 14 ++++++++++++-- 2 files changed, 24 insertions(+), 5 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb index e2e68b2ca..fcc44f9e3 100644 --- a/lib/puppet/provider/nameservice/directoryservice.rb +++ b/lib/puppet/provider/nameservice/directoryservice.rb @@ -206,9 +206,18 @@ class DirectoryService < Puppet::Provider::NameService if ensure_value == :present @resource.class.validproperties.each do |name| next if name == :ensure - next unless val = @resource.should(name) || autogen(name) - # JJM: This calls the method. - self.send(name.to_s + "=", val) + + # LAK: We use property.sync here rather than directly calling + # the settor method because the properties might do some kind + # of conversion. In particular, the user gid property might + # have a string and need to convert it to a number + if @resource.should(name) + @resource.property(name).sync + elsif value = autogen(name) + self.send(name.to_s + "=", value) + else + next + end end end end diff --git a/lib/puppet/provider/nameservice/netinfo.rb b/lib/puppet/provider/nameservice/netinfo.rb index 29600052b..ac7bc94b1 100644 --- a/lib/puppet/provider/nameservice/netinfo.rb +++ b/lib/puppet/provider/nameservice/netinfo.rb @@ -138,8 +138,18 @@ class NetInfo < Puppet::Provider::NameService if arg == :present @resource.class.validproperties.each do |name| next if name == :ensure - next unless val = @resource.should(name) || autogen(name) - self.send(name.to_s + "=", val) + + # LAK: We use property.sync here rather than directly calling + # the settor method because the properties might do some kind + # of conversion. In particular, the user gid property might + # have a string and need to convert it to a number + if @resource.should(name) + @resource.property(name).sync + elsif value = autogen(name) + self.send(name.to_s + "=", value) + else + next + end end end end -- cgit From 679fede620becbe7a9e934718ddf6420e28ec2cc Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 30 Sep 2008 17:51:10 -0500 Subject: Removing commented code from the user type from about 2005 Signed-off-by: Luke Kanies --- lib/puppet/type/user.rb | 23 ----------------------- 1 file changed, 23 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb index 79cd5ff26..571781d5a 100755 --- a/lib/puppet/type/user.rb +++ b/lib/puppet/type/user.rb @@ -191,29 +191,6 @@ module Puppet end end - # these three properties are all implemented differently on each platform, - # so i'm disabling them for now - - # FIXME Puppet::Property::UserLocked is currently non-functional - #newproperty(:locked) do - # desc "The expected return code. An error will be returned if the - # executed command returns something else." - #end - - # FIXME Puppet::Property::UserExpire is currently non-functional - #newproperty(:expire) do - # desc "The expected return code. An error will be returned if the - # executed command returns something else." - # @objectaddflag = "-e" - #end - - # FIXME Puppet::Property::UserInactive is currently non-functional - #newproperty(:inactive) do - # desc "The expected return code. An error will be returned if the - # executed command returns something else." - # @objectaddflag = "-f" - #end - newparam(:name) do desc "User name. While limitations are determined for each operating system, it is generally a good idea to keep to -- cgit From 2afc4f5f159870ae2b8af3fb30037716a659dcf4 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 30 Sep 2008 18:08:55 -0500 Subject: Adding tests for the user retrieve method Signed-off-by: Luke Kanies --- lib/puppet/type/user.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/type/user.rb b/lib/puppet/type/user.rb index 571781d5a..bb0a86fd0 100755 --- a/lib/puppet/type/user.rb +++ b/lib/puppet/type/user.rb @@ -272,7 +272,7 @@ module Puppet current_value = :absent if absent - prophash[property] = :absent + prophash[property] = :absent else current_value = property.retrieve prophash[property] = current_value @@ -280,7 +280,6 @@ module Puppet if property.name == :ensure and current_value == :absent absent = true -# next end prophash } -- cgit From ee579641f72b399e9e13e989a7779b533004b634 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 30 Sep 2008 22:04:38 -0500 Subject: Modified the behaviour of resource-level 'retrieve' -- it only calls 'retrieve' on each property if the resource exists. Signed-off-by: Luke Kanies --- lib/puppet/type.rb | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index c7a866e2c..1989fc057 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -906,14 +906,24 @@ class Type # get a hash of the current properties. def currentpropvalues(override_value = nil) - # it's important to use the method here, as it follows the order - # in which they're defined in the object + # it's important to use the 'properties' method here, as it follows the order + # in which they're defined in the object. It also guarantees that 'ensure' + # is the first property, which is important for skipping 'retrieve' on + # all the properties if the resource is absent. + ensure_state = false return properties().inject({}) { | prophash, property| - prophash[property] = override_value.nil? ? - property.retrieve : - override_value - prophash - } + if property.name == :ensure + ensure_state = property.retrieve + prophash[property] = ensure_state + else + if ensure_state == :absent + prophash[property] = :absent + else + prophash[property] = property.retrieve + end + end + prophash + } end # Are we running in noop mode? -- cgit From 7da41528e82e5b4d36d4ac33689db0fa92480b3f Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Tue, 30 Sep 2008 22:34:05 -0500 Subject: Modified the group and zone resource types to no longer call 'currentpropvalues' as a means of setting all values to absent. There should be no behaviour change from this change. Signed-off-by: Luke Kanies --- lib/puppet/type.rb | 16 +++++++++------- lib/puppet/type/group.rb | 8 -------- lib/puppet/type/zone.rb | 6 +++++- 3 files changed, 14 insertions(+), 16 deletions(-) (limited to 'lib/puppet') diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb index 1989fc057..e377a3547 100644 --- a/lib/puppet/type.rb +++ b/lib/puppet/type.rb @@ -901,17 +901,19 @@ class Type # retrieve the current value of all contained properties def retrieve - return currentpropvalues + return currentpropvalues end - # get a hash of the current properties. - def currentpropvalues(override_value = nil) - # it's important to use the 'properties' method here, as it follows the order - # in which they're defined in the object. It also guarantees that 'ensure' + # Get a hash of the current properties. Returns a hash with + # the actual property instance as the key and the current value + # as the, um, value. + def currentpropvalues + # It's important to use the 'properties' method here, as it follows the order + # in which they're defined in the class. It also guarantees that 'ensure' # is the first property, which is important for skipping 'retrieve' on # all the properties if the resource is absent. ensure_state = false - return properties().inject({}) { | prophash, property| + return properties().inject({}) do | prophash, property| if property.name == :ensure ensure_state = property.retrieve prophash[property] = ensure_state @@ -923,7 +925,7 @@ class Type end end prophash - } + end end # Are we running in noop mode? diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb index 2a5ac30da..cb11a60a4 100755 --- a/lib/puppet/type/group.rb +++ b/lib/puppet/type/group.rb @@ -118,14 +118,6 @@ module Puppet defaultto false end - - def retrieve - if self.provider and @provider.exists? - return super - else - return currentpropvalues(:absent) - end - end end end diff --git a/lib/puppet/type/zone.rb b/lib/puppet/type/zone.rb index 4fd92672c..7601ec47b 100644 --- a/lib/puppet/type/zone.rb +++ b/lib/puppet/type/zone.rb @@ -377,7 +377,11 @@ end result = setstatus(hash) result else - return currentpropvalues(:absent) + # Return all properties as absent. + return properties().inject({}) do | prophash, property| + prophash[property] = :absent + prophash + end end end -- cgit