Pavel Zuna wrote:
Rob Crittenden wrote:You're probably referring to the user plugin, where gecos and krbprincipalname defaults are set inside pre_callback. It's a residue from some time ago when we didn't use autofill=True with default_from and it didn't have any effect on optional parameters. It's a small change, but I included an updated version of the patch with this email.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: ad...@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: ad...@pzuna krblastpwdchange: 20090904122852Z krbpasswordexpiration: 20091203122852Z memberof: cn=admins,cn=groups,cn=accounts,dc=pzuna objectclass: top objectclass: person objectclass: posixaccount objectclass: krbprincipalaux objectclass: inetuserAdvantages: 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.gitIt 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.PavelWhy are you using a pre_callback() to define default values instead of using default_from? It seems clearer to use that.
Ok. I gather you've moved a lot of logic into the pre_callback plugin to avoid overriding execute? One other goal we wanted was to allow plugins to extend other plugins and this may be good step on the way there. So for example, a user wants to be able to set some extra objectclass, they could do it with a similar pre_callback extension to the user plugin (once we do the plumbing for it, that is).
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.I know, but I couldn't find a better solution. I thought we could add something like a 'real_name' kwarg to params, but this has 2 main disadvantages:1) it only makes sense with parameters that refer to an LDAP attribute 2) it doesn't work for attributes with no param counterpartsThe global set is a good idea as long as we consider only our own plugins. 3rd party plugins would have to modify this set to add their own attributes. Also, attributes might have a different meaning for different objects. They usually don't, but take 'cn' for example. We use it as an ID, name, full name (for users), etc.
Ok, that's a good point. I'm wondering if its overkill to write a registration system for this, something like:
def set_label(attr, label, context='') def get_label(attr, context='')In this we'd store a dict that would be keyed on attr+context. Some examples might be:
set_label('uid', 'Login')
set_label('gn', 'First Name')
set_label('cn', 'Full Name', context='user')
set_label('cn', 'Group Name', context='group')
The lookup would first look for a context-specific entry and fall back
to a non-context specific, something like:
if attr+context in labeldict: return labeldict[attr+context] elif attr in labeldict: return labeldict[attr] else return "woop" Like I said, might be overkill ;-)But still, the alarms go off when we're putting the same things in multiple places.
In the user plugin 'ou' is in the default attributes. We don't use this attribute.Since we don't use it, it's not going to get displayed anyway.All the values in attribute_names, default_attribute and attribute_order are subject to change. I set them to initial values I thought are acceptable, but I don't think I'm the right person to decide what's going to be there. Ideally, someone with more UI/text/user friendliness experience should review it (there's not programming knowledge required to change the values).
Ok, this particular attribute brings up DIT issues which is why I flagged it.
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.Yeah, there's no documentation at this point, but I'm working on it as we "speak".robOne more thing, I reviewed your latest patches and they make change to host and service plugins. Your patches should be pushed first, because they are more complex and also more important, so I posted updated versions to the corresponding email threads on freeipa-devel.Pavel
Ok, I'm not too worried about that, its always a game of catch-up :-) rob
smime.p7s
Description: S/MIME Cryptographic Signature
_______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
