On Fri, Feb 21, 2014 at 03:30:22PM +0100, Petr Viktorin wrote: > Hello, > A permission object was not removed in permission-add when adding > the ACI failed. Here is a fix. > > https://fedorahosted.org/freeipa/ticket/4187 > > > Earlier we agreed that patch authors should bug the reviewer. I > guess now this means I should set Patch-review-by in the ticket, > right? So: > Martin, you reviewed the other ACI patches so I think you should > continue. If you don't agree, unset the field in the ticket. > > -- > PetrĀ³
> From 5ad2066b71b09248d348a5c4c85ef2ace0c553a4 Mon Sep 17 00:00:00 2001 > From: Petr Viktorin <pvikt...@redhat.com> > Date: Fri, 21 Feb 2014 13:58:15 +0100 > Subject: [PATCH] permission_add: Remove permission entry if adding the ACI > fails > > https://fedorahosted.org/freeipa/ticket/4187 > --- > ipalib/plugins/permission.py | 15 ++++++++++++++- > ipatests/test_xmlrpc/test_permission_plugin.py | 25 +++++++++++++++++++++++++ > 2 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/ipalib/plugins/permission.py b/ipalib/plugins/permission.py > index > 64deb99ef98583daf0419a240aa8852b0262874d..cb6f18b478735920bbf6cef4febc91481631c560 > 100644 > --- a/ipalib/plugins/permission.py > +++ b/ipalib/plugins/permission.py > @@ -812,7 +812,20 @@ def pre_callback(self, ldap, dn, entry, attrs_list, > *keys, **options): > return dn > > def post_callback(self, ldap, dn, entry, *keys, **options): > - self.obj.add_aci(entry) > + try: > + self.obj.add_aci(entry) > + except Exception: > + # Adding the ACI failed. > + # We want to be 100% sure the ACI is not there, so try to > + # remove it. (This is a no-op if the ACI was not added.) > + self.obj.remove_aci(entry) > + # Remove the entry > + try: > + self.api.Backend['ldap2'].delete_entry(entry) > + except errors.NotFound: > + pass > + # Re-raise original exception > + raise > self.obj.postprocess_result(entry, options) > return dn I'm not totally happy about this patch. What happens when the ACI is already in LDAP and some part of that self.obj.add_aci(entry) operation fails? Won't you go and instead of doing noop, remove the ACI instead? -- Jan Pazdziora Principal Software Engineer, Identity Management Engineering, Red Hat _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel