[Freeipa-devel] Re: [PATCHES] Make plugins use baseldap classes.

[Rob Crittenden]

Pavel Zůna wrote:
> This is a series of patches that depends on patches:
> - Improve attribute printing in the CLI.
> - Improve ipalib.plugins.baseldap classes.
> 
> All plugins are converted to extend baseldap classes. This makes things 
> more consistent, fixes some general bugs (with return values for 
> example) and it also makes plugins easier to maintain (as it removes a 
> lot of duplicate code).
> 
> Because baseldap classes have features that enable us to define 
> relationships between plugins, I thought it might be best to submit all 
> of the conversions at once and have all the relationships fully defined.
> 
> Affected plugins:
> config
> user
> host
> service
> group
> hostgroup
> netgroup
> rolegroup
> taskgroup
> pwpolicy
> 
> There's also a patch that fixes all unit tests.
> 
> Jenny, I included you to Cc, because the patch introduces a lot of 
> changes to the UI (and you're probably not going to like me for this).
> 
> Each command extending the LDAP* base classes now has a --raw option. 
> Without it, data from LDAP is formated and translated to human readable. 
> For example:
> 
> # ipa user-show admin --all
> ----------
> user-show:
> ----------
> User: admin
>   user id: admin
>   full name: Administrator
>   last name: Administrator
>   home directory: /home/admin
>   login shell: /bin/bash
>   uid number: 999
>   gid number: 1001
>   gecos: Administrator
>   kerberos principal: admin at PZUNA
>   last password change: 20090904122852Z
>   password expiration: 20091203122852Z
>   member of groups: admins
> 
> # ipa user-show admin --all --raw
> ----------
> user-show:
> ----------
>   dn: uid=admin,cn=users,cn=accounts,dc=pzuna
>   uid: admin
>   cn: Administrator
>   sn: Administrator
>   homedirectory: /home/admin
>   loginshell: /bin/bash
>   uidnumber: 999
>   gidnumber: 1001
>   gecos: Administrator
>   krbprincipalname: admin at PZUNA
>   krblastpwdchange: 20090904122852Z
>   krbpasswordexpiration: 20091203122852Z
>   memberof: cn=admins,cn=groups,cn=accounts,dc=pzuna
>   objectclass: top
>   objectclass: person
>   objectclass: posixaccount
>   objectclass: krbprincipalaux
>   objectclass: inetuser
> 
> Advantages: more user friendly, allows for easy localization, 
> translation of DNs to primary keys (immediately usable as input to other 
> plugin commands)
> 
> I recommend, that you use the --raw flag for testing.
> 
> 
> 
> I know it's a lot of changes, so I setup a git repo at:
> git://fedorapeople.org/~pzuna/freeipa.git
> 
> It should be up-to-date with master and all my patches are applied 
> there. I hope it makes reviewing them at least a bit easier.
> 
> Pavel

Why are you using a pre_callback() to define default values instead of 
using default_from? It seems clearer to use that.

This also duplicates some values in the attribute_names() dictionary. I 
wonder if this can be either created from the parameters or define a 
global set somewhere that covers all plugins.

In the user plugin 'ou' is in the default attributes. We don't use this 
attribute.

I think that in general baseldap needs to be documented to explain how 
things work. There is no explanation for object_class_config, for 
example, that it defines the attribute in cn=ipaConfig that contains the 
default list of objectclasses for the object.

rob

-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 3245 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20090911/5cab0975/attachment.bin>