On 11/14/2013 03:11 PM, Martin Kosek wrote: > On 11/13/2013 04:56 PM, Ana Krivokapic wrote: >> On 11/13/2013 03:08 PM, Martin Kosek wrote: >>> On 10/29/2013 12:30 PM, Ana Krivokapic wrote: >>>> On 10/15/2013 06:09 PM, Ana Krivokapic wrote: >>>>> On 09/30/2013 10:02 AM, Petr Viktorin wrote: >>>>>> On 09/27/2013 03:12 PM, Martin Kosek wrote: >>>>>>> On 09/27/2013 03:00 PM, Jan Cholasta wrote: >>>>>>>> On 23.9.2013 19:41, Ana Krivokapic wrote: >>>>>>>>> On 09/19/2013 03:29 PM, Ana Krivokapic wrote: >>>>>>> ... >>>>>>>> Patch 69: >>>>>>>> >>>>>>>> I think the changes in the update file should be also done in the >>>>>>>> right LDIF >>>>>>>> files in install/share, though I don't know what is the recent >>>>>>>> consensus on this. >>>>>>>> >>>>>>>> >>>>>>>> Honza >>>>>>>> >>>>>>> Last time I checked, we used to do the change both in LDIF and update >>>>>>> file. Just to avoid the LDIF become obsolete. >>>>>>> >>>>>>> Martin >>>>>> Rob recently said his preference is to move everything from LDIF to >>>>>> updates, >>>>>> and out of the the LDIF files: >>>>>> http://www.redhat.com/archives/freeipa-devel/2013-September/msg00106.html >>>>>> >>>>>> I would agree, having two places with the same information is redundant >>>>>> and >>>>>> error-prone. >>>>>> >>>>> Thanks Honza for the review. >>>>> >>>>> I incorporated your suggestions in this updated patchset. I attached all >>>>> the >>>>> patches for more convenient reviewing, but only patches 68 and 70 have >>>>> changed. >>>>> >>>>> I haven't done any changes in the LDIF files since the consensus seems to >>>>> be not >>>>> to do that. >>>> Patch 70 needed a rebase, attaching the whole patchset again. >>> This works pretty fine, I have few comments though: >>> >>> 1) 0068: the task should be run only for users/hosts base DN - this is >>> where we >>> confine our automember and I think admin may be surprised that the rebuild >>> call >>> is does not respect it. >> Fixed. >> >>> 2) 0068: I am missing some examples for automember-rebuild in the help. At >>> least for running rebuild for all users/hosts and for running it for >>> specified >>> user/host. >> I added some examples, as well as a general description of the new command. >> >>> 3) 0068: I think that the labels/doc for the new command/options should be >>> improved. It is not obvious, that automember-rebuild can run for all >>> users/hosts, at least from following doc: >>> >>> # ipa help automember >>> ... >>> automember-rebuild Rebuild auto membership for specified >>> entries. >>> ... >>> >>> Maybe we should remove the "for specified entries" part? >>> >>> As for the options, we now have this: >>> >>> # ipa help automember-rebuild >>> Usage: ipa [global-options] automember-rebuild [options] >>> >>> Rebuild auto membership for specified entries. >>> Options: >>> -h, --help show this help message and exit >>> --type=['group', 'hostgroup'] >>> Grouping to which the rule applies <--completely >>> stray >>> --users=STR Users for which the rebuild task will be run >>> --hosts=STR Hosts for which the rebuild task will be run >>> >>> >>> We should probably also do not mention specified entries here. >>> >>> As for option help, maybe the following would better show that it can be run >>> for all entries? >>> >>> --type=['group', 'hostgroup'] >>> Rebuild membership for all members of a grouping >>> --users=STR Rebuild membership for specified users >>> --hosts=STR Rebuild membership for specified hosts >> Agreed, labels fixed as per your suggestions. >> >>> This makes me thinking we may want to forbid entering both --type and >>> --users/--hosts - i.e. either rebuild all or just selected ones - to make >>> the >>> selection even more clear. But I am open to discussion on this one. >> Validation prevents any invalid combination of options (e.g. --type=group and >> --hosts used together, or --type=hostgroup and --users used together). If, >> for >> example, --users is specified, then --type=group is allowed but not >> required. I >> think it's clear enough. >> >>> 4) 0069: Add Automember Export Updates Task is currently redundant. I think >>> we >>> should either have permissions for all 3 possible tasks or for just the one >>> we use. >> I removed the unused permission. >> >>> 5) 0069: permissions should be of SYSTEM type as the ACI is out of SUFFIX, >>> so >>> that user does not try to modify them (will be able to in future versions). >>> Adding Petr3 to CC for heads up on this one. >> Fixed. >> >>> Martin >> Thanks for the review, the updated patchset is attached. > Looks good. Last thing I would like to get fixed is this part in 0068: > > + types = { > + 'group': ('user', 'users', 'users'), > + 'hostgroup': ('host', 'hosts', 'computers'), > + } > + > + obj_name, opt_name, dn_part = types[gtype] > + basedn = DN(('cn', dn_part),('cn', 'accounts'), api.env.basedn) > + obj = self.api.Object[obj_name] > > I know it works now, but if we sometime decide to maybe support different user > containers and not have it in cn=users,cn=accounts, it will help not user > hardcoded DNs like that, but rather standard > > DN(api.env.container_users, api.env.basedn) > or > DN(api.env.container_hosts, api.env.basedn) > > Martin
Fixed, updated patch attached. -- Regards, Ana Krivokapic Associate Software Engineer FreeIPA team Red Hat Inc.
From 861d2bb86a09c5e346642bbaa5a8246261fc9ed1 Mon Sep 17 00:00:00 2001 From: Ana Krivokapic <[email protected]> Date: Thu, 19 Sep 2013 14:01:58 +0200 Subject: [PATCH] Add automember rebuild command Add a new command to IPA CLI: ipa automember-rebuild The command integrates the automember rebuild membership task functionality into IPA CLI. It makes it possible to rebuild automember membership for groups/hostgroups. Design: http://www.freeipa.org/page/V3/Automember_rebuild_membership https://fedorahosted.org/freeipa/ticket/3752 --- API.txt | 9 +++ VERSION | 2 +- ipalib/plugins/automember.py | 143 ++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 143 insertions(+), 11 deletions(-) diff --git a/API.txt b/API.txt index cddb9d719f38dd3d89e2633148311e437aed0bc9..b3b3a7e381bfea01b17dbf6ac05c159ebe61928f 100644 --- a/API.txt +++ b/API.txt @@ -199,6 +199,15 @@ command: automember_mod output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None)) output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None) output: Output('value', <type 'unicode'>, None) +command: automember_rebuild +args: 0,4,3 +option: Str('hosts*') +option: StrEnum('type', cli_name='type', multivalue=False, required=False, values=(u'group', u'hostgroup')) +option: Str('users*') +option: Str('version?', exclude='webui') +output: Output('result', <type 'bool'>, None) +output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None) +output: Output('value', <type 'unicode'>, None) command: automember_remove_condition args: 1,8,5 arg: Str('cn', cli_name='automember_rule') diff --git a/VERSION b/VERSION index 32f6efbc4d4768c77c514a3367cb9feb039205e5..0dacb97041d903733d3005045cac850e21f56d65 100644 --- a/VERSION +++ b/VERSION @@ -89,4 +89,4 @@ IPA_DATA_VERSION=20100614120000 # # ######################################################## IPA_API_VERSION_MAJOR=2 -IPA_API_VERSION_MINOR=66 +IPA_API_VERSION_MINOR=67 diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py index 4f563f11953dafac0d5bfd00f0309aca2f346f81..fc696cc62ba97f6ff7c8682ac2b7e6eca1d0bd92 100644 --- a/ipalib/plugins/automember.py +++ b/ipalib/plugins/automember.py @@ -16,13 +16,11 @@ # # You should have received a copy of the GNU General Public License # along with this program. If not, see <http://www.gnu.org/licenses/>. - -from ipalib import api, errors -from ipalib import Str, StrEnum +import uuid +import ldap as _ldap +from ipalib import api, errors, Str, StrEnum, _, ngettext from ipalib.plugins.baseldap import * -from ipalib import _, ngettext from ipalib.request import context -import ldap as _ldap from ipapython.dn import DN __doc__ = _(""" @@ -43,6 +41,8 @@ match any rule. In case of user entries this group will be a fallback group because all users are by default members of group specified in IPA config. +The automember-rebuild command can be used to retroactively run automember rules +against existing entries, thus rebuilding their membership. EXAMPLES: @@ -107,6 +107,18 @@ Delete an automember rule: ipa automember-del --type=hostgroup webservers ipa automember-del --type=group devel + + Rebuild membership for all users: + ipa automember-rebuild --type=group + + Rebuild membership for all hosts: + ipa automember-rebuild --type=hostgroup + + Rebuild membership for specified users: + ipa automember-rebuild --users=tuser1 --users=tuser2 + + Rebuild membership for specified hosts: + ipa automember-rebuild --hosts=web1.example.com --hosts=web2.example.com """) # Options used by Condition Add and Remove. @@ -184,14 +196,17 @@ class automember(LDAPObject): ), ) - def dn_exists(self, grouptype, groupname, *keys): + def dn_exists(self, otype, oname): ldap = self.api.Backend.ldap2 - dn = self.api.Object[grouptype].get_dn(groupname) + dn = self.api.Object[otype].get_dn(oname) try: - (gdn, entry_attrs) = ldap.get_entry(dn, []) + entry = ldap.get_entry(dn, []) except errors.NotFound: - raise errors.NotFound(reason=_(u'Group: %s not found!') % groupname) - return gdn + raise errors.NotFound( + reason=_(u'%(otype)s "%(oname)s" not found') % + dict(otype=otype, oname=oname) + ) + return entry.dn def get_dn(self, *keys, **options): if self.parent_object: @@ -587,3 +602,111 @@ def execute(self, *keys, **options): return result api.register(automember_default_group_show) + + +class automember_rebuild(Command): + __doc__ = _('Rebuild auto membership.') + # TODO: Add a --dry-run option: + # https://fedorahosted.org/freeipa/ticket/3936 + takes_options = ( + group_type[0].clone( + required=False, + label=_('Rebuild membership for all members of a grouping') + ), + Str( + 'users*', + label=_('Users'), + doc=_('Rebuild membership for specified users'), + ), + Str( + 'hosts*', + label=_('Hosts'), + doc=_('Rebuild membership for specified hosts'), + ), + ) + has_output = output.standard_value + msg_summary = _('Automember rebuild membership task completed') + + def validate(self, **kw): + """ + Validation rules: + - at least one of 'type', 'users', 'hosts' is required + - 'users' and 'hosts' cannot be combined together + - if 'users' and 'type' are specified, 'type' must be 'group' + - if 'hosts' and 'type' are specified, 'type' must be 'hostgroup' + """ + super(automember_rebuild, self).validate(**kw) + users, hosts, gtype = kw.get('users'), kw.get('hosts'), kw.get('type') + + if not (gtype or users or hosts): + raise errors.MutuallyExclusiveError( + reason=_('at least one of options: type, users, hosts must be ' + 'specified') + ) + + if users and hosts: + raise errors.MutuallyExclusiveError( + reason=_("users and hosts cannot both be set") + ) + if gtype == 'group' and hosts: + raise errors.MutuallyExclusiveError( + reason=_("hosts cannot be set when type is 'group'") + ) + if gtype == 'hostgroup' and users: + raise errors.MutuallyExclusiveError( + reason=_("users cannot be set when type is 'hostgroup'") + ) + + def execute(self, *keys, **options): + ldap = self.api.Backend.ldap2 + cn = str(uuid.uuid4()) + + gtype = options.get('type') + if not gtype: + gtype = 'group' if options.get('users') else 'hostgroup' + + types = { + 'group': ( + 'user', + 'users', + DN(api.env.container_user, api.env.basedn) + ), + 'hostgroup': ( + 'host', + 'hosts', + DN(api.env.container_host, api.env.basedn) + ), + } + + obj_name, opt_name, basedn = types[gtype] + obj = self.api.Object[obj_name] + + names = options.get(opt_name) + if names: + for name in names: + obj.get_dn_if_exists(name) + search_filter = ldap.make_filter_from_attr( + obj.primary_key.name, + names, + rules=ldap.MATCH_ANY + ) + else: + search_filter = '(%s=*)' % obj.primary_key.name + + entry = ldap.make_entry( + DN( + ('cn', cn), + ('cn', 'automember rebuild membership'), + ('cn', 'tasks'), + ('cn', 'config'), + ), + objectclass=['top', 'extensibleObject'], + cn=[cn], + basedn=[basedn], + filter=[search_filter], + scope=['sub'] + ) + ldap.add_entry(entry) + return dict(result=True, value=u'') + +api.register(automember_rebuild) -- 1.8.3.1
_______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
