From 8a534bf07b55b20566c50211c9f90d638aead3da Mon Sep 17 00:00:00 2001 From: Rob Crittenden Date: Mon, 13 Dec 2010 11:10:37 -0500 Subject: Give the memberof plugin time to work when adding/removing reverse members. When we add/remove reverse members it looks like we're operating on group A but we're really operating on group B. This adds/removes the member attribute on group B and the memberof plugin adds the memberof attribute into group A. We need to give the memberof plugin a chance to do its work so loop a few times, reading the entry to see if the number of memberof is more or less what we expect. Bail out if it is taking too long. ticket 560 --- ipalib/errors.py | 17 ++++++++++++ ipalib/plugins/baseldap.py | 65 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/ipalib/errors.py b/ipalib/errors.py index f99a3a45a..a06e5bcce 100644 --- a/ipalib/errors.py +++ b/ipalib/errors.py @@ -1158,6 +1158,23 @@ class ManagedGroupExistsError(ExecutionError): errno = 4024 format = _('Unable to create private group. Group \'%(group)s\' already exists.') + +class ReverseMemberError(ExecutionError): + """ + **4025** Raised when verifying that all reverse members have been added or removed. + + For example: + + >>> raise ReverseMemberError(verb=u'added', exc=u'Group \'foo\' not found.') + Traceback (most recent call last): + ... + A problem was encounted when verifying that all members were added: Group 'foo' not found. + """ + + errno = 4025 + format = _('A problem was encounted when verifying that all members were %(verb)s: %(exc)s') + + class BuiltinError(ExecutionError): """ **4100** Base class for builtin execution errors (*4100 - 4199*). diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py index 3299f8015..d010cd982 100644 --- a/ipalib/plugins/baseldap.py +++ b/ipalib/plugins/baseldap.py @@ -22,8 +22,9 @@ Base classes for LDAP plugins. import re import json +import time -from ipalib import crud, errors +from ipalib import api, crud, errors from ipalib import Method, Object from ipalib import Flag, Int, List, Str from ipalib.base import NameSpace @@ -175,6 +176,50 @@ def get_effective_rights(ldap, dn, attrs=None): return rdict +def wait_for_memberof(keys, entry_start, completed, show_command, adding=True): + """ + When adding or removing reverse members we are faking an update to + object A by updating the member attribute in object B. The memberof + plugin makes this work by adding or removing the memberof attribute + to/from object A, it just takes a little bit of time. + + This will loop for 6+ seconds, retrieving object A so we can see + if all the memberof attributes have been updated. + """ + if completed == 0: + # nothing to do + return api.Command[show_command](keys[-1])['result'] + + if 'memberof' in entry_start: + starting_memberof = len(entry_start['memberof']) + else: + starting_memberof = 0 + + # Loop a few times to give the memberof plugin a chance to add the + # entries. Don't sleep for more than 6 seconds. + memberof = 0 + x = 0 + while x < 20: + # sleep first because the first search, even on a quiet system, + # almost always fails to have memberof set. + time.sleep(.3) + x = x + 1 + + # FIXME: put a try/except around here? I think it is probably better + # to just let the exception filter up to the caller. + entry_attrs = api.Command[show_command](keys[-1])['result'] + if 'memberof' in entry_attrs: + memberof = len(entry_attrs['memberof']) + + if adding: + if starting_memberof + completed >= memberof: + break + else: + if starting_memberof + completed <= memberof: + break + + return entry_attrs + class LDAPObject(Object): """ Object representing a LDAP entry. @@ -1326,6 +1371,9 @@ class LDAPAddReverseMember(LDAPModReverseMember): else: attrs_list = self.obj.default_attributes + # Pull the record as it is now so we can know how many members + # there are. + entry_start = self.api.Command[self.show_command](keys[-1])['result'] completed = 0 failed = {'member': {self.reverse_attr: []}} for attr in options.get(self.reverse_attr, []): @@ -1351,7 +1399,11 @@ class LDAPAddReverseMember(LDAPModReverseMember): except errors.PublicError, e: failed['member'][self.reverse_attr].append((attr, unicode(msg))) - entry_attrs = self.api.Command[self.show_command](keys[-1])['result'] + # Wait for the memberof plugin to update the entry + try: + entry_attrs = wait_for_memberof(keys, entry_start, completed, self.show_command, adding=True) + except Exception, e: + raise errors.ReverseMemberError(verb=_('added'), exc=str(e)) for callback in self.POST_CALLBACKS: if hasattr(callback, 'im_self'): @@ -1429,6 +1481,9 @@ class LDAPRemoveReverseMember(LDAPModReverseMember): else: attrs_list = self.obj.default_attributes + # Pull the record as it is now so we can know how many members + # there are. + entry_start = self.api.Command[self.show_command](keys[-1])['result'] completed = 0 failed = {'member': {self.reverse_attr: []}} for attr in options.get(self.reverse_attr, []): @@ -1454,7 +1509,11 @@ class LDAPRemoveReverseMember(LDAPModReverseMember): except errors.PublicError, e: failed['member'][self.reverse_attr].append((attr, unicode(msg))) - entry_attrs = self.api.Command[self.show_command](keys[-1])['result'] + # Wait for the memberof plugin to update the entry + try: + entry_attrs = wait_for_memberof(keys, entry_start, completed, self.show_command, adding=False) + except Exception, e: + raise errors.ReverseMemberError(verb=_('removed'), exc=str(e)) for callback in self.POST_CALLBACKS: if hasattr(callback, 'im_self'): -- cgit