On Tue, Nov 25, 2008 at 9:27 PM, Luke Kanies <[EMAIL PROTECTED]> wrote: > > I know James has already commented on and pushed these (a bit > prematurely, he admits), but here are my comments.
heh. I was hoping to get comments first :) commit: http://github.com/nigelkersten/puppet/commit/eb27418a0a9afcb7c342231f3c810c13d5b60e1b in my features/0.24.x/1770 branch. patch inline at end of mail for comments. >> + ds_value = dscl_plist[key] >> + if not @@ds_to_ns_attribute_map[ds_attribute] >> == :members # only members uses arrays so far >> + ds_value = ds_value[0] >> + end > > Shouldn't this just be !=...? actually picked up a gid/uid bug here, changed and fixed.. >> + newvalue(:absent) do >> + Puppet.notice "prop ensure = absent" >> + provider.delete >> + end >> + end > > I assume this is extra debugging lying around... Fixed in above commit. > And you're missing docs for the :ensure property, and, of course, the > params below should have their docs extended a bit. Fixed in above commit. >> module Puppet >> newtype(:group) do >> - @doc = "Manage groups. This type can only create groups. <snip> > > This whole doc string is heinously out of date and should be > rewritten. It apparently hasn't been updated since providers were > developed. > > I'd remove any mention of who does what, since the providers should > cover that. > > If you need help rewriting it, let me know. :) I messed up generating this patch for the mailing list. The committed patch has an updated description. http://github.com/jamtur01/puppet/tree/0.24.x/lib%2Fpuppet/type/group.rb but have now removed all mention of provider specifics. >> + newproperty(:members, :array_matching >> => :all, :required_features => :manages_members) do >> + desc "The members of the group. For directory services >> where group >> + membership is stored in the group objects, not the >> users." > > Nice -- you've picked up a lot of the tricks. :) I need to poke around and work out what the utility of declaring 'events' in properties is... but yeah, I'm starting to feel like I'm picking up some of the way providers work... > I'll save commentary on the tests for the next version -- seems like > there'll be some refactoring anyway. Yep. I agree the whole nameservice situation needs a rethink. There's too much complexity that can be easily avoided. patch: >From 8c04703a792c47c8cb6a261e07cfa5e1db4de12f Mon Sep 17 00:00:00 2001 From: Nigel Kersten <[EMAIL PROTECTED]> Date: Wed, 26 Nov 2008 07:54:32 -0800 Subject: [PATCH] fix bug with numeric uid/gid in directoryservice provider. doc string cleanups --- .../provider/nameservice/directoryservice.rb | 16 +++++++++++++- lib/puppet/type/computer.rb | 8 ++++-- lib/puppet/type/group.rb | 21 +++---------------- 3 files changed, 23 insertions(+), 22 deletions(-) diff --git a/lib/puppet/provider/nameservice/directoryservice.rb b/lib/puppet/provider/nameservice/directoryservice.rb index 230da39..8b1df7d 100644 --- a/lib/puppet/provider/nameservice/directoryservice.rb +++ b/lib/puppet/provider/nameservice/directoryservice.rb @@ -147,8 +147,20 @@ class DirectoryService < Puppet::Provider::NameService ds_attribute = key.sub("dsAttrTypeStandard:", "") next unless (@@ds_to_ns_attribute_map.keys.include?(ds_attribute) and type_properties.include? @@ds_to_ns_attribute_map[ds_attribute]) ds_value = dscl_plist[key] - if not @@ds_to_ns_attribute_map[ds_attribute] == :members # only members uses arrays so far - ds_value = ds_value[0] + case @@ds_to_ns_attribute_map[ds_attribute] + when :members: + ds_value = ds_value # only members uses arrays so far + when :gid, :uid: + # OS X stores objects like uid/gid as strings. + # Try casting to an integer for these cases to be + # consistent with the other providers and the group type + # validation + begin + ds_value = Integer(ds_value[0]) + rescue ArgumentError + ds_value = ds_value[0] + end + else ds_value = ds_value[0] end attribute_hash[@@ds_to_ns_attribute_map[ds_attribute]] = ds_value end diff --git a/lib/puppet/type/computer.rb b/lib/puppet/type/computer.rb index 0c0a709..ccbcadf 100644 --- a/lib/puppet/type/computer.rb +++ b/lib/puppet/type/computer.rb @@ -29,23 +29,25 @@ Puppet::Type.newtype(:computer) do end newproperty(:ensure, :parent => Puppet::Property::Ensure) do + desc "Control the existences of this computer record. Set this attribute to + ``present`` to ensure the computer record exists. Set it to ``absent`` + to delete any computer records with this name" newvalue(:present) do provider.create end newvalue(:absent) do - Puppet.notice "prop ensure = absent" provider.delete end end newparam(:name) do - desc "The " + desc "The authoritative 'short' name of the computer record." isnamevar end newparam(:realname) do - desc "realname" + desc "The 'long' name of the computer record." end newproperty(:en_address) do diff --git a/lib/puppet/type/group.rb b/lib/puppet/type/group.rb index 1167962..e3507ad 100755 --- a/lib/puppet/type/group.rb +++ b/lib/puppet/type/group.rb @@ -1,10 +1,3 @@ -# Manage Unix groups. This class is annoyingly complicated; There -# is some variety in whether systems use 'groupadd' or 'addgroup', but OS X -# significantly complicates the picture by using NetInfo. Eventually we -# will also need to deal with systems that have their groups hosted elsewhere -# (e.g., in LDAP). That will likely only be a problem for OS X, since it -# currently does not use the POSIX interfaces, since lookupd's cache screws -# things up. require 'etc' require 'facter' @@ -14,16 +7,10 @@ module Puppet @doc = "Manage groups. On most platforms this can only create groups. Group membership must be managed on individual users. - On OS X, group membership is managed as an attribute of the group. - This resource type uses the prescribed native tools for creating - groups and generally uses POSIX APIs for retrieving information - about them. It does not directly modify ``/etc/group`` or anything. - - For most platforms, the tools used are ``groupadd`` and its ilk; - for Mac OS X, dscl/dseditgroup are used. - - This is currently unconfigurable, but if you desperately need it - to be so, please contact us." + On some platforms such as OS X, group membership is managed as an + attribute of the group, not the user record. Providers must have + the feature 'manages_members' to manage the 'members' property of + a group record." feature :manages_members, "For directories where membership is an attribute of groups not users." -- 1.5.6.4 --~--~---------~--~----~------------~-------~--~----~ You received this message because you are subscribed to the Google Groups "Puppet Developers" group. To post to this group, send email to puppet-dev@googlegroups.com To unsubscribe from this group, send email to [EMAIL PROTECTED] For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en -~----------~----~----~----~------~----~------~--~---
