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 <akriv...@redhat.com>
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
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to