Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Tue, 2011-08-23 at 17:43 -0400, Rob Crittenden wrote:
...
Looks lots better, just a couple of nits:
* The default-group api has type as an arg and everywhere else it is
--type, can we make it consistent? We can argue about this with Martin
tomorrow if you'd like.
I suggested this API to JR when he had difficulties using IPA command
without an argument, i.e. ipa automember-default-group-remove
--type=group. I wanted to keep the automember-default-group-* commands
consistent compared to having automember-default-group-{remove, show}
TYPE as an argument and automember-default-group-add with TYPE as an
option.
Martin
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
JR Aquino wrote: On Aug 19, 2011, at 2:16 AM, Martin Kosek wrote: Hi JR, I get to your plugin again. You can see my findings below. On Tue, 2011-08-09 at 22:41 +, JR Aquino wrote: ... Ok New Patch attached. I believe this addresses the above. 1. Requires(pre): 389-ds-base= 1.2.9.5-1 1) Please, remove the change to FreeIPA spec, its no longer needed since we shipped version 2.1 and it already requires sufficient 389-ds-base version. Done. 2. replica-automember.ldif added for dsinstance to install during replica installs: +dn: cn=Auto Membership Plugin,cn=plugins,cn=config +changetype: modify +add: nsslapd-pluginConfigArea +nsslapd-pluginConfigArea: cn=automember,cn=etc,$SUFFIX 2) OK. I would do it a bit different - have one LDIF for nsslapd-pluginConfigArea setting and second for creating the base automember structure. Master would then use both LDIFs and a replica both of them. We would then be without duplicates in LDIF. But your way acceptable. Please allow the 2 ldif's in as they are. I tried to split them to leverage cn=config change in common, however, I encountered a 389 ds bug. I will be opening a bug with Nathan in BZ to address the bug. If you feel strongly, we can either: A: Accept the two LDIFs as is and revisit after a newer version of 389 ds is available. B: Wait until 389 ds addresses the bug and make the minor modification you suggested above. 3. autoMemberScope is now set for each: groups: cn=users,cn=accounts,$SUFFIX hostgroups: cn=computers,cn=accounts,$SUFFIX OK 4. Corrected examples Set the default target group: ipa automember-default-group-set --default-group=webservers hostgroup ipa automember-default-group-set --default-group=ipausers group Set the default target group: ipa automember-default-group-remove hostgroup ipa automember-default-group-remove group Show the default target group: ipa automember-default-group-show hostgroup ipa automember-default-group-show group 5. Corrected examples Add a condition to the rule: ipa automember-add-condition --key=fqdn --type=hostgroup --inclusive-regex=^web[1-9+]\.example\.com webservers 3) Please fix the regex to ^web[1-9]+\.example\.com. I think its just a mistake - right now for example a host web11.example.com does not match. Fixed ipa automember-add-condition --key=manager --type=group --inclusive-regex=^mscott admins 4) I think you wanted to use devel rule instead of non-existent admins automember rule. You are correct, this has been fixed. Add an exclusive condition to the rule to prevent auto asignment: ipa automember-add-condition --key=fqdn --type=hostgroup --exclusive-regex=^web5\.example\.com webservers Remove a condition from the rule: ipa automember-remove-condition --key=fqdn --type=hostgroup --inclusive-regex=^www[1-9+]\.example\.com webservers 5) The same as in 3) Fixed 6. Correct bug for adding duplicate conditions. Included test for it in the test suite. OK. Here are my additional findings: 6) There some more example commands in doc which are not complete and require some user typing: Display a automember rule: ipa automember-show webservers Delete an automember rule: ipa automember-del webservers Grouping type option is missing Fixed. Added the appropriate flags in the examples 7) I get internal error when running examples from the automember doc: # ipa automember-add --type=group devel - Added automember rule devel - Automember Rule: devel # ipa automember-add-condition --key=manager --type=group --inclusive-regex=^mscott admins ipa: ERROR: an internal error has occurred Fixed. That's all. The plugin gets better with every version, I think we may soon be ready for pushing - when all of the issues are resolved. Martin Please let me know how it looks now. Looks lots better, just a couple of nits: * The default-group api has type as an arg and everywhere else it is --type, can we make it consistent? We can argue about this with Martin tomorrow if you'd like. * The tests focus mainly on bucket allocation, it also needs to test adding/removing conditions and rules. I wonder if there should actually be two test suites, one to test the basics of the plugin and one to make sure it operates properly when creating entries. * Can you document in the ldifs and the installer why there are separate ones for master and replicas (for dsinstance.py I think you can just say # see ldifs for details). rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
Hi JR, I get to your plugin again. You can see my findings below. On Tue, 2011-08-09 at 22:41 +, JR Aquino wrote: ... Ok New Patch attached. I believe this addresses the above. 1. Requires(pre): 389-ds-base = 1.2.9.5-1 1) Please, remove the change to FreeIPA spec, its no longer needed since we shipped version 2.1 and it already requires sufficient 389-ds-base version. 2. replica-automember.ldif added for dsinstance to install during replica installs: +dn: cn=Auto Membership Plugin,cn=plugins,cn=config +changetype: modify +add: nsslapd-pluginConfigArea +nsslapd-pluginConfigArea: cn=automember,cn=etc,$SUFFIX 2) OK. I would do it a bit different - have one LDIF for nsslapd-pluginConfigArea setting and second for creating the base automember structure. Master would then use both LDIFs and a replica both of them. We would then be without duplicates in LDIF. But your way acceptable. 3. autoMemberScope is now set for each: groups: cn=users,cn=accounts,$SUFFIX hostgroups: cn=computers,cn=accounts,$SUFFIX OK 4. Corrected examples Set the default target group: ipa automember-default-group-set --default-group=webservers hostgroup ipa automember-default-group-set --default-group=ipausers group Set the default target group: ipa automember-default-group-remove hostgroup ipa automember-default-group-remove group Show the default target group: ipa automember-default-group-show hostgroup ipa automember-default-group-show group 5. Corrected examples Add a condition to the rule: ipa automember-add-condition --key=fqdn --type=hostgroup --inclusive-regex=^web[1-9+]\.example\.com webservers 3) Please fix the regex to ^web[1-9]+\.example\.com. I think its just a mistake - right now for example a host web11.example.com does not match. ipa automember-add-condition --key=manager --type=group --inclusive-regex=^mscott admins 4) I think you wanted to use devel rule instead of non-existent admins automember rule. Add an exclusive condition to the rule to prevent auto asignment: ipa automember-add-condition --key=fqdn --type=hostgroup --exclusive-regex=^web5\.example\.com webservers Remove a condition from the rule: ipa automember-remove-condition --key=fqdn --type=hostgroup --inclusive-regex=^www[1-9+]\.example\.com webservers 5) The same as in 3) 6. Correct bug for adding duplicate conditions. Included test for it in the test suite. OK. Here are my additional findings: 6) There some more example commands in doc which are not complete and require some user typing: Display a automember rule: ipa automember-show webservers Delete an automember rule: ipa automember-del webservers Grouping type option is missing 7) I get internal error when running examples from the automember doc: # ipa automember-add --type=group devel - Added automember rule devel - Automember Rule: devel # ipa automember-add-condition --key=manager --type=group --inclusive-regex=^mscott admins ipa: ERROR: an internal error has occurred That's all. The plugin gets better with every version, I think we may soon be ready for pushing - when all of the issues are resolved. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Mon, 2011-08-08 at 16:16 +, JR Aquino wrote: On Aug 8, 2011, at 2:04 AM, Martin Kosek wrote: On Fri, 2011-08-05 at 18:36 +, JR Aquino wrote: ~ Jr Aquino, GCIH | Information Security Specialist Citrix Online | 7408 Hollister Avenue | Goleta, CA 93117 T: +1 805.690.3478 jr.aqu...@citrixonline.com http://www.citrixonline.com On Aug 2, 2011, at 5:55 AM, Rob Crittenden wrote: JR Aquino wrote: On Aug 1, 2011, at 5:56 AM, Rob Crittenden wrote: Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Fri, 2011-08-05 at 18:36 +, JR Aquino wrote: ~ Jr Aquino, GCIH | Information Security Specialist Citrix Online | 7408 Hollister Avenue | Goleta, CA 93117 T: +1 805.690.3478 jr.aqu...@citrixonline.com http://www.citrixonline.com On Aug 2, 2011, at 5:55 AM, Rob Crittenden wrote: JR Aquino wrote: On Aug 1, 2011, at 5:56 AM, Rob Crittenden wrote: Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Fri, 2011-08-05 at 00:03 +, JR Aquino wrote: On Aug 3, 2011, at 7:32 AM, Rob Crittenden wrote: JR Aquino wrote: On Aug 2, 2011, at 5:55 AM, Rob Crittendenrcrit...@redhat.com wrote: JR Aquino wrote: I am fairly opposed to removing 'default' attrs which the rules are applied to... I am happy to provide a means to override them. While it may be second nature for all of us to know that there is an fqdn attribute, etc, our consumers are likely not going to intrinsically know our schema. We also deliberately mask the real attribute names in the framework. (fqdn = Host name) Providing a default feels like a happy medium which allows for ease of use and somewhat of a safety belt against users defining an incorrect attribute name. It also might get somewhat tiring to constantly provide --key=fqdn every time you add a hostname regex? Ok, but when you display rules fqdn is displayed. How are users to know they shouldn't include fqdn= when removing existing rules? I guess my preference would be to heavily document, in the example, the plugin, and the docs... My concern is that without a default, a typo in the attr would produce unintended results. Without a schema checker, it's kinda tough to take an attr at face value from a user. Does the python ldap implementation have a means to check schema in order to verify an attribute? The design of the automember pluginhHaving the attr in the Regex does make for some complexity We do have a schema checker. You can test for existence of an attribute with something like: import ldap as _ldap obj = ldap.schema.get_obj(_ldap.schema.AttributeType, attr) if obj is None: # Error, no such attribute def check_attr(self, attr): Verify that the user supplied key is a valid attribute in the schema ldap = self.api.Backend.ldap2 obj = ldap.schema.get_obj(_ldap.schema.AttributeType, attr) return obj [Thu Aug 04 16:58:41 2011] [error] File /usr/lib/python2.7/site-packages/ipalib/plugins/automember.py, line 209, in check_attr [Thu Aug 04 16:58:41 2011] [error] obj = ldap.schema.get_obj(_ldap.schema.AttributeType, attr) [Thu Aug 04 16:58:41 2011] [error] AttributeError: 'NoneType' object has no attribute 'get_obj' Seems that ldap doesn't have a get_obj inside of schema? This error means that schema attribute in the ldap2 object is None. I think the schema may not be loaded yet, there is a lazy retrieval of the LDAP schema. You can either use the following method in ldap2.py instead: def get_syntax(self, attr, value): if not self.schema: self.get_schema() obj = self.schema.get_obj(_ldap.schema.AttributeType, attr) if obj is not None: return obj.syntax else: return None or add the lazy retrieval function calling yourself: f not self.schema: self.get_schema() Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Aug 3, 2011, at 7:32 AM, Rob Crittenden wrote: JR Aquino wrote: On Aug 2, 2011, at 5:55 AM, Rob Crittendenrcrit...@redhat.com wrote: JR Aquino wrote: I am fairly opposed to removing 'default' attrs which the rules are applied to... I am happy to provide a means to override them. While it may be second nature for all of us to know that there is an fqdn attribute, etc, our consumers are likely not going to intrinsically know our schema. We also deliberately mask the real attribute names in the framework. (fqdn = Host name) Providing a default feels like a happy medium which allows for ease of use and somewhat of a safety belt against users defining an incorrect attribute name. It also might get somewhat tiring to constantly provide --key=fqdn every time you add a hostname regex? Ok, but when you display rules fqdn is displayed. How are users to know they shouldn't include fqdn= when removing existing rules? I guess my preference would be to heavily document, in the example, the plugin, and the docs... My concern is that without a default, a typo in the attr would produce unintended results. Without a schema checker, it's kinda tough to take an attr at face value from a user. Does the python ldap implementation have a means to check schema in order to verify an attribute? The design of the automember pluginhHaving the attr in the Regex does make for some complexity We do have a schema checker. You can test for existence of an attribute with something like: import ldap as _ldap obj = ldap.schema.get_obj(_ldap.schema.AttributeType, attr) if obj is None: # Error, no such attribute def check_attr(self, attr): Verify that the user supplied key is a valid attribute in the schema ldap = self.api.Backend.ldap2 obj = ldap.schema.get_obj(_ldap.schema.AttributeType, attr) return obj [Thu Aug 04 16:58:41 2011] [error] File /usr/lib/python2.7/site-packages/ipalib/plugins/automember.py, line 209, in check_attr [Thu Aug 04 16:58:41 2011] [error] obj = ldap.schema.get_obj(_ldap.schema.AttributeType, attr) [Thu Aug 04 16:58:41 2011] [error] AttributeError: 'NoneType' object has no attribute 'get_obj' Seems that ldap doesn't have a get_obj inside of schema? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Aug 2, 2011, at 5:55 AM, Rob Crittenden rcrit...@redhat.com wrote: JR Aquino wrote: I am fairly opposed to removing 'default' attrs which the rules are applied to... I am happy to provide a means to override them. While it may be second nature for all of us to know that there is an fqdn attribute, etc, our consumers are likely not going to intrinsically know our schema. We also deliberately mask the real attribute names in the framework. (fqdn = Host name) Providing a default feels like a happy medium which allows for ease of use and somewhat of a safety belt against users defining an incorrect attribute name. It also might get somewhat tiring to constantly provide --key=fqdn every time you add a hostname regex? Ok, but when you display rules fqdn is displayed. How are users to know they shouldn't include fqdn= when removing existing rules? I guess my preference would be to heavily document, in the example, the plugin, and the docs... My concern is that without a default, a typo in the attr would produce unintended results. Without a schema checker, it's kinda tough to take an attr at face value from a user. Does the python ldap implementation have a means to check schema in order to verify an attribute? The design of the automember pluginhHaving the attr in the Regex does make for some complexity ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
JR Aquino wrote: On Aug 2, 2011, at 5:55 AM, Rob Crittendenrcrit...@redhat.com wrote: JR Aquino wrote: I am fairly opposed to removing 'default' attrs which the rules are applied to... I am happy to provide a means to override them. While it may be second nature for all of us to know that there is an fqdn attribute, etc, our consumers are likely not going to intrinsically know our schema. We also deliberately mask the real attribute names in the framework. (fqdn = Host name) Providing a default feels like a happy medium which allows for ease of use and somewhat of a safety belt against users defining an incorrect attribute name. It also might get somewhat tiring to constantly provide --key=fqdn every time you add a hostname regex? Ok, but when you display rules fqdn is displayed. How are users to know they shouldn't include fqdn= when removing existing rules? I guess my preference would be to heavily document, in the example, the plugin, and the docs... My concern is that without a default, a typo in the attr would produce unintended results. Without a schema checker, it's kinda tough to take an attr at face value from a user. Does the python ldap implementation have a means to check schema in order to verify an attribute? The design of the automember pluginhHaving the attr in the Regex does make for some complexity We do have a schema checker. You can test for existence of an attribute with something like: import ldap as _ldap obj = ldap.schema.get_obj(_ldap.schema.AttributeType, attr) if obj is None: # Error, no such attribute rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Mon, 2011-08-01 at 19:11 +, JR Aquino wrote: On Aug 1, 2011, at 5:56 AM, Rob Crittenden wrote: Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. There is no mod to change types, you
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Aug 1, 2011, at 11:28 PM, Martin Kosek mko...@redhat.com wrote: On Mon, 2011-08-01 at 19:11 +, JR Aquino wrote: On Aug 1, 2011, at 5:56 AM, Rob Crittenden wrote: Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. There is no mod to change types, you have to delete and re-add. automember-add Add an automember rule --type=ENUM
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Tue, 2011-08-02 at 07:25 +, JR Aquino wrote: On Aug 1, 2011, at 11:28 PM, Martin Kosek mko...@redhat.com wrote: On Mon, 2011-08-01 at 19:11 +, JR Aquino wrote: On Aug 1, 2011, at 5:56 AM, Rob Crittenden wrote: Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
JR Aquino wrote: On Aug 1, 2011, at 5:56 AM, Rob Crittenden wrote: Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. There is no mod to change types, you have to delete and re-add. automember-add Add an automember rule --type=ENUM(hostgroup, group) --desc=STR description of this auto membership rule --inclusive-regex=LIST Inclusive Regex --exclusive-regex=LIST Exclusive
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
JR Aquino wrote:
On Aug 1, 2011, at 11:28 PM, Martin Kosekmko...@redhat.com wrote:
I made all of the small adjustments and then nearly literally stared frustrated
at the code for 6 hours today.
I see no way of accomplishing the feat of replicating the ldapmodmember
treatment of failed attributes, without overriding LDAPUpdate's exec
LDAPUpdate is simply looking to return entrys_attrs as the result, and only a
dn for the callbacks... I can't set or send anything back to track the faileds
in a callback.
You can see it's been bothering me, as it is midnight my time and I'm thinking
about it still. :/
Overriding execute is ok, we just didn't like duplicating all the code.
If you want to override the return value do something like:
def my_function(LDAPUpdate):
has_output = (
output.Entry('result'),
output.Output('failed',
type=dict,
doc=_('Members that could not be added'),
),
)
def execute(self, *keys, **options):
failed = dict('inclusive': [], 'exclusive', [])
/* calculate failed */
/* Make options consist of the values you want updated */
result = super(my_function, self).execute(*keys, **options)
return(result=result, failed=failed, value=keys[-1])
You may have to tweak failed so it looks ok on Output. The add/remove
member methods in baseldap.py may provide some additional guidance.
rob
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Aug 2, 2011, at 1:09 AM, Martin Kosek wrote: On Tue, 2011-08-02 at 07:25 +, JR Aquino wrote: On Aug 1, 2011, at 11:28 PM, Martin Kosek mko...@redhat.com wrote: On Mon, 2011-08-01 at 19:11 +, JR Aquino wrote: On Aug 1, 2011, at 5:56 AM, Rob Crittenden wrote: Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. There is no mod to change types, you have to delete and re-add. automember-add Add an automember rule --type=ENUM(hostgroup, group)
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. There is no mod to change types, you have to delete and re-add. automember-add Add an automember rule --type=ENUM (hostgroup, group) --desc=STR description of this auto membership rule --inclusive-regex=LIST Inclusive Regex --exclusive-regex=LIST Exclusive Regex automember-add-condition Add conditions to automember rule
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Aug 1, 2011, at 5:56 AM, Rob Crittenden wrote: Martin Kosek wrote: On Sat, 2011-07-30 at 00:54 +, JR Aquino wrote: On Jul 21, 2011, at 8:53 AM, JR Aquino wrote: On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. There is no mod to change types, you have to delete and re-add. automember-add Add an automember rule --type=ENUM(hostgroup, group) --desc=STR description of this auto
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. There is no mod to change types, you have to delete and re-add. automember-add Add an automember rule --type=ENUM (hostgroup, group) --desc=STRdescription of this auto membership rule --inclusive-regex=LISTInclusive Regex --exclusive-regex=LISTExclusive Regex automember-add-condition Add conditions to automember rule --inclusive-regex=LISTInclusive Regex --exclusive-regex=LISTExclusive Regex automember-del Delete an automember rule
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Thu, 2011-07-21 at 10:31 -0400, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. Yes, this will make more flexible API for the future. We will be able to implement new membership types easily if we want to. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. I think we have to leave the type in the DN as it is now, i.e. 1) cn=Hostgroups,cn=automembership,cn=etc,$SUFFIX 2) cn=Groups,cn=automembership,cn=etc,$SUFFIX 3) ... Otherwise we wouldn't be able to
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Jul 21, 2011, at 7:31 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-21 at 03:37 +, JR Aquino wrote: Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Great, preparing the command parameters in pre_callback is much cleaner. Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. I agree with Rob here, this will make the code better. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. Yes, it is wise :-) But the error message is really not clear to the user. We should tell him that there must be at least one inclusive rule. I wonder if we shouldn't force user to create a hostgroupclarity object with at least one inclusive rule and than make sure that in all operations at least one inclusive rule stays here. Or we could delete the empty LDAP object after the last inclusive rule is removed, as we do with DNS record LDAP objects in dnsrecord-del. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foorulename- in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? I think the best shot would be to create a new command and overload the execute method in that case. Like in hbacrule_enable. You would be able to set dn correctly here and do the update. Does it makes sense? Rob? Martin I agree. We are better off abstracting things now so we can get the API right. I think we can stick more or less with the command names, just in a new plugin and some new arguments. I see the plugin with the following methods: Each takes a single argument, the name of the rule. I don't think I'd stick type into the DN so you wouldn't be able to use the same rule name for different object types. If we want to allow that then we'd need to add --type to a lot more commands. There is no mod to change types, you have to delete and re-add. automember-add Add an automember rule --type=ENUM (hostgroup, group) --desc=STR description of this auto membership rule --inclusive-regex=LIST Inclusive Regex --exclusive-regex=LIST Exclusive Regex automember-add-condition Add conditions to automember rule
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
JR Aquino wrote: On Jul 15, 2011, at 7:55 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-14 at 23:05 +, JR Aquino wrote: On Jul 14, 2011, at 11:55 AM, wrote: https://fedorahosted.org/freeipa/ticket/1272 * Added new container in etc to hold the automembership configs. * Modified constants to point to the new container * Modified dsinstance to create the container * Modified hostgroup.py to add the new commands * Added xmlrpc test to verify functionality Minor adjustment: Auto Membership Plugin isn't available until 1.2.9-0.2+ Modified freeipa.spec.in: BuildRequires: 389-ds-base-devel= 1.2.9-0.2 I have reviewed your patch. Basic functionality is OK but I have some concerns. 1) I am not sure with the command name, it is not really clear to me what this command does. But I know from my experience that inventing a cool name for something new may be the most difficult task at all :-) Maybe command name hostgrouprule or hostgroupauto would be more clear? Perhaps my example docs were too soft with fqdn=^www[1-9]+\.example\.com etc.. I should 'clarify'... perhaps what I need to do is add more useful information to the doc, for example If I were to add to the doc area examples where hostnames are like: w[1-9]+s2r8\.example\.com The real reason for the usefulness of this technology, is in SaaS, Cloud, and Cluster environments, where the hostnames tend to be non-human readable, and more like a serial number detailing their function, their rack location, or their vm-instance, etc... It is because of those scenarios that caused me so much grief as a security engineer trying to assign rights that it became clear that I could just define the reproducible pattern to match assignment into a host group. The hostnames needed clarity in order to understand where they belonged in the network. I'll give it one more chance to pass the censors since I've been internally calling it clarity for the last 2 1/2 years that I've been using it... 2) Overloading execute method in functions hostgroupclarity_add_condition and hostgroupclarity_remove_condition is an over-kill for me. I think we could just read current inclusive/exclusive regexes in pre_callback, modify them and let LDAPUpdate class do the standard LDAP operations. I'll recode to perform the actions in a pre_callback. 3) I miss hostgroupclarity-mod module. What would I do if I want to update Description? Thank you for catching this, I will add it. 4) I didn't like this construct in the code, its error prone to potential future parameter changes. +if len(options) == 2: # 'all' and 'raw' are always sent +raise errors.EmptyModlist() I know it's in baseldap.py but I still wouldn't like to see this in plugins. I should be able to omit that once the code is located in the pre_callback. 5) Test test_clarityrule_plugin.py: reference to inexistent python module: +Test the `ipalib/plugins/clarityrule.py` module. Thank you, that is left over from a previous attempt. I will remove it. Then I did some real testing of the new command: 6) Invalid examples, fqdn is not supposed to be a part of regex $ ipa hostgroupclarity-add --inclusive-hostname-regex=fqdn=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=fqdn=^www[1-9]+.example.com Also an oversight, thanks, I will correct it. 7) It does not make sense to have a rule with only an exclusive regex: $ ipa hostgroupclarity-add --exclusive-hostname-regex=^www5+\.example\.com webservers Hostgroup Clarity Rule: webservers $ ipa host-add --force foo.example.co $ ipa hostgroup-show webservers Host-group: webservers Description: Web Servers Member hosts: www1.example.com I think we should 1) hide exclusive regex option in hostgroupclarity-add and 2) check that there is at least one inclusive regex in the rule when running hostgroupclarity-add-condition and hostgroupclarity-remove-condition. I agree, I'll hide it during the creation, and force it to require an inclusive prior to adding an exclusive. 8) Plugin incorrectly handles a situation when both inclusive and exclusive regex-es are being added: $ ipa hostgroupclarity-add --inclusive-hostname-regex=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com $ ipa hostgroupclarity-add-condition --inclusive-hostname-regex=^web[1-9]+\.example\.com --exclusive-hostname-regex=www5\.example\.com webservers Inclusive Regex: fqdn=^web[1-9]+.example.com, fqdn=^www[1-9]+.example.com Exclusive Regex: www5.example.com Exclusive regex misses fqdn. Will look into this. 9) Removing multiple conditions also works incorrectly: $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com, fqdn=^web[1-9]+.example.com Exclusive Regex: fqdn=www5.example.com $ ipa
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Jul 20, 2011, at 8:37 AM, Rob Crittenden wrote: JR Aquino wrote: On Jul 15, 2011, at 7:55 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-14 at 23:05 +, JR Aquino wrote: On Jul 14, 2011, at 11:55 AM, wrote: https://fedorahosted.org/freeipa/ticket/1272 * Added new container in etc to hold the automembership configs. * Modified constants to point to the new container * Modified dsinstance to create the container * Modified hostgroup.py to add the new commands * Added xmlrpc test to verify functionality Minor adjustment: Auto Membership Plugin isn't available until 1.2.9-0.2+ Modified freeipa.spec.in: BuildRequires: 389-ds-base-devel= 1.2.9-0.2 I have reviewed your patch. Basic functionality is OK but I have some concerns. 1) I am not sure with the command name, it is not really clear to me what this command does. But I know from my experience that inventing a cool name for something new may be the most difficult task at all :-) Maybe command name hostgrouprule or hostgroupauto would be more clear? Perhaps my example docs were too soft with fqdn=^www[1-9]+\.example\.com etc.. I should 'clarify'... perhaps what I need to do is add more useful information to the doc, for example If I were to add to the doc area examples where hostnames are like: w[1-9]+s2r8\.example\.com The real reason for the usefulness of this technology, is in SaaS, Cloud, and Cluster environments, where the hostnames tend to be non-human readable, and more like a serial number detailing their function, their rack location, or their vm-instance, etc... It is because of those scenarios that caused me so much grief as a security engineer trying to assign rights that it became clear that I could just define the reproducible pattern to match assignment into a host group. The hostnames needed clarity in order to understand where they belonged in the network. I'll give it one more chance to pass the censors since I've been internally calling it clarity for the last 2 1/2 years that I've been using it... 2) Overloading execute method in functions hostgroupclarity_add_condition and hostgroupclarity_remove_condition is an over-kill for me. I think we could just read current inclusive/exclusive regexes in pre_callback, modify them and let LDAPUpdate class do the standard LDAP operations. I'll recode to perform the actions in a pre_callback. 3) I miss hostgroupclarity-mod module. What would I do if I want to update Description? Thank you for catching this, I will add it. 4) I didn't like this construct in the code, its error prone to potential future parameter changes. +if len(options) == 2: # 'all' and 'raw' are always sent +raise errors.EmptyModlist() I know it's in baseldap.py but I still wouldn't like to see this in plugins. I should be able to omit that once the code is located in the pre_callback. 5) Test test_clarityrule_plugin.py: reference to inexistent python module: +Test the `ipalib/plugins/clarityrule.py` module. Thank you, that is left over from a previous attempt. I will remove it. Then I did some real testing of the new command: 6) Invalid examples, fqdn is not supposed to be a part of regex $ ipa hostgroupclarity-add --inclusive-hostname-regex=fqdn=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=fqdn=^www[1-9]+.example.com Also an oversight, thanks, I will correct it. 7) It does not make sense to have a rule with only an exclusive regex: $ ipa hostgroupclarity-add --exclusive-hostname-regex=^www5+\.example\.com webservers Hostgroup Clarity Rule: webservers $ ipa host-add --force foo.example.co $ ipa hostgroup-show webservers Host-group: webservers Description: Web Servers Member hosts: www1.example.com I think we should 1) hide exclusive regex option in hostgroupclarity-add and 2) check that there is at least one inclusive regex in the rule when running hostgroupclarity-add-condition and hostgroupclarity-remove-condition. I agree, I'll hide it during the creation, and force it to require an inclusive prior to adding an exclusive. 8) Plugin incorrectly handles a situation when both inclusive and exclusive regex-es are being added: $ ipa hostgroupclarity-add --inclusive-hostname-regex=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com $ ipa hostgroupclarity-add-condition --inclusive-hostname-regex=^web[1-9]+\.example\.com --exclusive-hostname-regex=www5\.example\.com webservers Inclusive Regex: fqdn=^web[1-9]+.example.com, fqdn=^www[1-9]+.example.com Exclusive Regex: www5.example.com Exclusive regex misses fqdn. Will look into this. 9) Removing multiple conditions also works incorrectly: $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule:
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
Rob, I'm afraid I believe that ldap lookup is necessary. The user inputs a standard string to represent the possible host group… If i simply perform a get_dn it will indeed provide a dn, however, it won't verify that the host group actually exists… (you don't want to create an assignment rule for a non existent target host group) Martin, (except for the name Clarity), I have addressed your observations in this latest patch. Could you please have a look and let me know if there is anything else I need to take care of? Good point about the LDAP lookup. This looks a lot better but there are still a few issues: If group_dn is in the object then you can use self.obj.handle_not_found(*keys) for the NotFound. Ok, I will give that a shot! Or if it can't be moved, in the calls to group_dn() you can use the ldap handle passed into pre_callback. I guess you are using the includetype tuple to avoid coding long variable names everywhere? Would a symbol be better, eg: INCLUDE_RE = 'automemberinclusiveregex' EXCLUDE_RE = 'automemberexclusiveregex' That works, I'll swap em. Is there a way to validate the regex? Now that you mention it, I believe if I import re, we should be able to validate the initial regex and raise an exception if it is bogus. If we were to add an equivalent user group handler would it be the same code in add_condition and remove_condition? It is sort of nice to have everything together at the moment, I suspect it will need to be generalized at some point. Well. For the groups, I was thinking it starts to get a little different. I would still reuse the condition, but I believe I would pivot users into groups based upon something like their manager? Adding a clarity with no rules won't let you add rules: # ipa hostgroup-add --desc=hg1 hg1 # ipa hostgroupclarity-add hg1 # ipa hostgroupclarity-add-condition --exclusive-hostname-regex=^web5\.example\.com hg1 ipa: ERROR: no modifications to be performed This ^ is deliberate, you cannot add an exclusion rule if there is no existing or simultaneous inclusive rule. :) Martin asked for that, and I think its wise. The way you explained clarity today in IRC, how it brings clarity to managing membership with names no human can grok, I got it. Still, clarity is a bit awkward as a name. automember might be a better choice. Fair enough ;) I tried, perhaps I can /allude/ to it in the help / docs. automember it is. One final class I have been struggling with that I want to add… The object and attribute which defines the 'default group' is the parent of the actual rules… i.e. cn=hostgroup,cn=automember,cn=etc… The ipa cli seems to only want to let me make mods that assume I am specifying a target object on the cli… ipa hostgroupautomember-default-group=foo rulename - in this scenario, we don't actually want or need a rule name because its the container above… I have had success making the writes, but the cli syntax just doesn't lend itself to that level of abstraction… Any suggestions? ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Jul 15, 2011, at 7:55 AM, Rob Crittenden wrote: Martin Kosek wrote: On Thu, 2011-07-14 at 23:05 +, JR Aquino wrote: On Jul 14, 2011, at 11:55 AM, wrote: https://fedorahosted.org/freeipa/ticket/1272 * Added new container in etc to hold the automembership configs. * Modified constants to point to the new container * Modified dsinstance to create the container * Modified hostgroup.py to add the new commands * Added xmlrpc test to verify functionality Minor adjustment: Auto Membership Plugin isn't available until 1.2.9-0.2+ Modified freeipa.spec.in: BuildRequires: 389-ds-base-devel= 1.2.9-0.2 I have reviewed your patch. Basic functionality is OK but I have some concerns. 1) I am not sure with the command name, it is not really clear to me what this command does. But I know from my experience that inventing a cool name for something new may be the most difficult task at all :-) Maybe command name hostgrouprule or hostgroupauto would be more clear? Perhaps my example docs were too soft with fqdn=^www[1-9]+\.example\.com etc.. I should 'clarify'... perhaps what I need to do is add more useful information to the doc, for example If I were to add to the doc area examples where hostnames are like: w[1-9]+s2r8\.example\.com The real reason for the usefulness of this technology, is in SaaS, Cloud, and Cluster environments, where the hostnames tend to be non-human readable, and more like a serial number detailing their function, their rack location, or their vm-instance, etc... It is because of those scenarios that caused me so much grief as a security engineer trying to assign rights that it became clear that I could just define the reproducible pattern to match assignment into a host group. The hostnames needed clarity in order to understand where they belonged in the network. I'll give it one more chance to pass the censors since I've been internally calling it clarity for the last 2 1/2 years that I've been using it... 2) Overloading execute method in functions hostgroupclarity_add_condition and hostgroupclarity_remove_condition is an over-kill for me. I think we could just read current inclusive/exclusive regexes in pre_callback, modify them and let LDAPUpdate class do the standard LDAP operations. I'll recode to perform the actions in a pre_callback. 3) I miss hostgroupclarity-mod module. What would I do if I want to update Description? Thank you for catching this, I will add it. 4) I didn't like this construct in the code, its error prone to potential future parameter changes. +if len(options) == 2: # 'all' and 'raw' are always sent +raise errors.EmptyModlist() I know it's in baseldap.py but I still wouldn't like to see this in plugins. I should be able to omit that once the code is located in the pre_callback. 5) Test test_clarityrule_plugin.py: reference to inexistent python module: +Test the `ipalib/plugins/clarityrule.py` module. Thank you, that is left over from a previous attempt. I will remove it. Then I did some real testing of the new command: 6) Invalid examples, fqdn is not supposed to be a part of regex $ ipa hostgroupclarity-add --inclusive-hostname-regex=fqdn=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=fqdn=^www[1-9]+.example.com Also an oversight, thanks, I will correct it. 7) It does not make sense to have a rule with only an exclusive regex: $ ipa hostgroupclarity-add --exclusive-hostname-regex=^www5+\.example\.com webservers Hostgroup Clarity Rule: webservers $ ipa host-add --force foo.example.co $ ipa hostgroup-show webservers Host-group: webservers Description: Web Servers Member hosts: www1.example.com I think we should 1) hide exclusive regex option in hostgroupclarity-add and 2) check that there is at least one inclusive regex in the rule when running hostgroupclarity-add-condition and hostgroupclarity-remove-condition. I agree, I'll hide it during the creation, and force it to require an inclusive prior to adding an exclusive. 8) Plugin incorrectly handles a situation when both inclusive and exclusive regex-es are being added: $ ipa hostgroupclarity-add --inclusive-hostname-regex=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com $ ipa hostgroupclarity-add-condition --inclusive-hostname-regex=^web[1-9]+\.example\.com --exclusive-hostname-regex=www5\.example\.com webservers Inclusive Regex: fqdn=^web[1-9]+.example.com, fqdn=^www[1-9]+.example.com Exclusive Regex: www5.example.com Exclusive regex misses fqdn. Will look into this. 9) Removing multiple conditions also works incorrectly: $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com, fqdn=^web[1-9]+.example.com Exclusive Regex:
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Thu, 2011-07-14 at 23:05 +, JR Aquino wrote: On Jul 14, 2011, at 11:55 AM, wrote: https://fedorahosted.org/freeipa/ticket/1272 * Added new container in etc to hold the automembership configs. * Modified constants to point to the new container * Modified dsinstance to create the container * Modified hostgroup.py to add the new commands * Added xmlrpc test to verify functionality Minor adjustment: Auto Membership Plugin isn't available until 1.2.9-0.2+ Modified freeipa.spec.in: BuildRequires: 389-ds-base-devel = 1.2.9-0.2 I have reviewed your patch. Basic functionality is OK but I have some concerns. 1) I am not sure with the command name, it is not really clear to me what this command does. But I know from my experience that inventing a cool name for something new may be the most difficult task at all :-) Maybe command name hostgrouprule or hostgroupauto would be more clear? 2) Overloading execute method in functions hostgroupclarity_add_condition and hostgroupclarity_remove_condition is an over-kill for me. I think we could just read current inclusive/exclusive regexes in pre_callback, modify them and let LDAPUpdate class do the standard LDAP operations. 3) I miss hostgroupclarity-mod module. What would I do if I want to update Description? 4) I didn't like this construct in the code, its error prone to potential future parameter changes. +if len(options) == 2: # 'all' and 'raw' are always sent +raise errors.EmptyModlist() I know it's in baseldap.py but I still wouldn't like to see this in plugins. 5) Test test_clarityrule_plugin.py: reference to inexistent python module: +Test the `ipalib/plugins/clarityrule.py` module. Then I did some real testing of the new command: 6) Invalid examples, fqdn is not supposed to be a part of regex $ ipa hostgroupclarity-add --inclusive-hostname-regex=fqdn=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=fqdn=^www[1-9]+.example.com 7) It does not make sense to have a rule with only an exclusive regex: $ ipa hostgroupclarity-add --exclusive-hostname-regex=^www5+\.example\.com webservers Hostgroup Clarity Rule: webservers $ ipa host-add --force foo.example.co $ ipa hostgroup-show webservers Host-group: webservers Description: Web Servers Member hosts: www1.example.com I think we should 1) hide exclusive regex option in hostgroupclarity-add and 2) check that there is at least one inclusive regex in the rule when running hostgroupclarity-add-condition and hostgroupclarity-remove-condition. 8) Plugin incorrectly handles a situation when both inclusive and exclusive regex-es are being added: $ ipa hostgroupclarity-add --inclusive-hostname-regex=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com $ ipa hostgroupclarity-add-condition --inclusive-hostname-regex=^web[1-9]+\.example\.com --exclusive-hostname-regex=www5\.example\.com webservers Inclusive Regex: fqdn=^web[1-9]+.example.com, fqdn=^www[1-9]+.example.com Exclusive Regex: www5.example.com Exclusive regex misses fqdn. 9) Removing multiple conditions also works incorrectly: $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com, fqdn=^web[1-9]+.example.com Exclusive Regex: fqdn=www5.example.com $ ipa hostgroupclarity-remove-condition webservers --inclusive-hostname-regex=^www[1-9]+\.example\.com --exclusive-hostname-regex=www5\.example\.com Inclusive Regex: fqdn=^web[1-9]+.example.com $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^web[1-9]+.example.com Exclusive Regex: fqdn=www5.example.com 10) When removing nonexistent regex I would expect more explaining error message: $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^web[1-9]+.example.com Exclusive Regex: fqdn=www5.example.com $ ipa hostgroupclarity-remove-condition webservers --inclusive-hostname-regex=foo ipa: ERROR: no modifications to be performed Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
Martin Kosek wrote: On Thu, 2011-07-14 at 23:05 +, JR Aquino wrote: On Jul 14, 2011, at 11:55 AM, wrote: https://fedorahosted.org/freeipa/ticket/1272 * Added new container in etc to hold the automembership configs. * Modified constants to point to the new container * Modified dsinstance to create the container * Modified hostgroup.py to add the new commands * Added xmlrpc test to verify functionality Minor adjustment: Auto Membership Plugin isn't available until 1.2.9-0.2+ Modified freeipa.spec.in: BuildRequires: 389-ds-base-devel= 1.2.9-0.2 I have reviewed your patch. Basic functionality is OK but I have some concerns. 1) I am not sure with the command name, it is not really clear to me what this command does. But I know from my experience that inventing a cool name for something new may be the most difficult task at all :-) Maybe command name hostgrouprule or hostgroupauto would be more clear? 2) Overloading execute method in functions hostgroupclarity_add_condition and hostgroupclarity_remove_condition is an over-kill for me. I think we could just read current inclusive/exclusive regexes in pre_callback, modify them and let LDAPUpdate class do the standard LDAP operations. 3) I miss hostgroupclarity-mod module. What would I do if I want to update Description? 4) I didn't like this construct in the code, its error prone to potential future parameter changes. +if len(options) == 2: # 'all' and 'raw' are always sent +raise errors.EmptyModlist() I know it's in baseldap.py but I still wouldn't like to see this in plugins. 5) Test test_clarityrule_plugin.py: reference to inexistent python module: +Test the `ipalib/plugins/clarityrule.py` module. Then I did some real testing of the new command: 6) Invalid examples, fqdn is not supposed to be a part of regex $ ipa hostgroupclarity-add --inclusive-hostname-regex=fqdn=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=fqdn=^www[1-9]+.example.com 7) It does not make sense to have a rule with only an exclusive regex: $ ipa hostgroupclarity-add --exclusive-hostname-regex=^www5+\.example\.com webservers Hostgroup Clarity Rule: webservers $ ipa host-add --force foo.example.co $ ipa hostgroup-show webservers Host-group: webservers Description: Web Servers Member hosts: www1.example.com I think we should 1) hide exclusive regex option in hostgroupclarity-add and 2) check that there is at least one inclusive regex in the rule when running hostgroupclarity-add-condition and hostgroupclarity-remove-condition. 8) Plugin incorrectly handles a situation when both inclusive and exclusive regex-es are being added: $ ipa hostgroupclarity-add --inclusive-hostname-regex=^www[1-9]+\.example\.com webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com $ ipa hostgroupclarity-add-condition --inclusive-hostname-regex=^web[1-9]+\.example\.com --exclusive-hostname-regex=www5\.example\.com webservers Inclusive Regex: fqdn=^web[1-9]+.example.com, fqdn=^www[1-9]+.example.com Exclusive Regex: www5.example.com Exclusive regex misses fqdn. 9) Removing multiple conditions also works incorrectly: $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^www[1-9]+.example.com, fqdn=^web[1-9]+.example.com Exclusive Regex: fqdn=www5.example.com $ ipa hostgroupclarity-remove-condition webservers --inclusive-hostname-regex=^www[1-9]+\.example\.com --exclusive-hostname-regex=www5\.example\.com Inclusive Regex: fqdn=^web[1-9]+.example.com $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^web[1-9]+.example.com Exclusive Regex: fqdn=www5.example.com 10) When removing nonexistent regex I would expect more explaining error message: $ ipa hostgroupclarity-show webservers Hostgroup Clarity Rule: webservers Inclusive Regex: fqdn=^web[1-9]+.example.com Exclusive Regex: fqdn=www5.example.com $ ipa hostgroupclarity-remove-condition webservers --inclusive-hostname-regex=foo ipa: ERROR: no modifications to be performed I think that with group_dn() you should use the api to get the entry rather than calling LDAP directly (I'd stick it into the clarity object). This is untested but I think it will work: def hostgroup_dn(self, hostgroup): entry = self.api.Command.user_show(hostgroup, all=True)['result'] return entry['dn'] rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
https://fedorahosted.org/freeipa/ticket/1272 * Added new container in etc to hold the automembership configs. * Modified constants to point to the new container * Modified dsinstance to create the container * Modified hostgroup.py to add the new commands * Added xmlrpc test to verify functionality binfWm24aLDHv.bin Description: freeipa-jraquino-0034-Create-FreeIPA-CLI-Plugin-for-the-389-Auto-Membershi.patch ~ Jr Aquino, GCIH | Information Security Specialist Citrix Online | 7408 Hollister Avenue | Goleta, CA 93117 T: +1 805.690.3478 jr.aqu...@citrixonline.com http://www.citrixonline.com ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 34 Create FreeIPA CLI Plugin for the 389 Auto Membership plugin
On Jul 14, 2011, at 11:55 AM, wrote: https://fedorahosted.org/freeipa/ticket/1272 * Added new container in etc to hold the automembership configs. * Modified constants to point to the new container * Modified dsinstance to create the container * Modified hostgroup.py to add the new commands * Added xmlrpc test to verify functionality Minor adjustment: Auto Membership Plugin isn't available until 1.2.9-0.2+ Modified freeipa.spec.in: BuildRequires: 389-ds-base-devel = 1.2.9-0.2 bin5faXeU99Xs.bin Description: freeipa-jraquino-0034-Create-FreeIPA-CLI-Plugin-for-the-389-Auto-Membershi.patch ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
