On 06/07/2016 10:42 AM, Martin Basti wrote:



On 07.06.2016 10:43, Jan Cholasta wrote:
On 7.6.2016 10:22, Martin Basti wrote:


On 07.06.2016 09:07, Jan Cholasta wrote:
On 6.6.2016 18:29, Martin Basti wrote:


On 03.06.2016 14:28, Stanislav Laznicka wrote:
On 06/03/2016 02:19 PM, Martin Basti wrote:

On 03.06.2016 14:13, Stanislav Laznicka wrote:
https://fedorahosted.org/freeipa/ticket/5892


NACK

please remove it from LDAPAddReverseMember too, it contains the same
code

Martin^2

Please see the modified patch.

Standa

ACK

Pushed to master: c56d65b064e1e0410c03cf1206816cad4d8d86cc

I think the attrs_list was supposed to be passed to the
ldap.get_entry() call rather than removed, which would fix that every
reverse member command always acts like --all was specified.

I'm really afraid, what can happen if we put attr_list into get_entry()
instead of '*', because this code were there for 4 years and I don't
feel happy enough to change it now, what we may break.

Should I revert this commit then and postpone the ticket?

It's a bug and should be fixed. The fix is easy so I see no point in postponing it. I see no reason to be really afraid, I'm pretty sure that removing the objectclass attribute (which is invisible in the CLI anyway) from the output of all the 4 commands that use this code won't break anything.


Ok
It seems that tests expect objectClass to be always returned in derived methods. Is that expected behavior? If so, please see the attached patch. I wonder if the keys of the passed options should make it to attrs_list as well (similarly to LDAPUpdate and LDAPCreate)?
From d554d96135160c8adcd3c798876de1e5ae83b4a0 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Tue, 7 Jun 2016 12:11:39 +0200
Subject: [PATCH 1/2] Revert "Removed dead code from
 LDAP{Remove,Add}ReverseMember"

While the code was really dead, it should serve a purpose elsewhere.
This reverts commit c56d65b064e1e0410c03cf1206816cad4d8d86cc.
---
 ipaserver/plugins/baseldap.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index 7367c87988bfa6f1db5c38c6dd633e541f59d27e..62b726da1a0baef9fc9fa4bb386a2101ca1f10b2 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -2131,6 +2131,14 @@ class LDAPAddReverseMember(LDAPModReverseMember):
             dn = callback(self, ldap, dn, *keys, **options)
             assert isinstance(dn, DN)
 
+        if options.get('all', False):
+            attrs_list = ['*'] + self.obj.default_attributes
+        else:
+            attrs_list = set(self.obj.default_attributes)
+            if options.get('no_members', False):
+                attrs_list.difference_update(self.obj.attribute_members)
+            attrs_list = list(attrs_list)
+
         completed = 0
         failed = {'member': {self.reverse_attr: []}}
         for attr in options.get(self.reverse_attr) or []:
@@ -2222,6 +2230,14 @@ class LDAPRemoveReverseMember(LDAPModReverseMember):
             dn = callback(self, ldap, dn, *keys, **options)
             assert isinstance(dn, DN)
 
+        if options.get('all', False):
+            attrs_list = ['*'] + self.obj.default_attributes
+        else:
+            attrs_list = set(self.obj.default_attributes)
+            if options.get('no_members', False):
+                attrs_list.difference_update(self.obj.attribute_members)
+            attrs_list = list(attrs_list)
+
         completed = 0
         failed = {'member': {self.reverse_attr: []}}
         for attr in options.get(self.reverse_attr) or []:
-- 
2.5.5

From 5a3c58f27efe73bcf478051d502df246ed10d0d2 Mon Sep 17 00:00:00 2001
From: Stanislav Laznicka <slazn...@redhat.com>
Date: Tue, 7 Jun 2016 13:51:46 +0200
Subject: [PATCH 2/2] The LDAP*ReverseMember shouldn't imply --all is always
 specified

The LDAP*ReverseMember methods would always return the whole LDAP
object even though --all is not specified.

https://fedorahosted.org/freeipa/ticket/5892
---
 ipaserver/plugins/baseldap.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ipaserver/plugins/baseldap.py b/ipaserver/plugins/baseldap.py
index 62b726da1a0baef9fc9fa4bb386a2101ca1f10b2..22eea887c4425bf890e0dc0da9e8779812fe5769 100644
--- a/ipaserver/plugins/baseldap.py
+++ b/ipaserver/plugins/baseldap.py
@@ -2159,7 +2159,7 @@ class LDAPAddReverseMember(LDAPModReverseMember):
                 failed['member'][self.reverse_attr].append((attr, unicode(e)))
 
         # Update the member data.
-        entry_attrs = ldap.get_entry(dn, ['*'])
+        entry_attrs = ldap.get_entry(dn, attrs_list + ['objectclass'])
         self.obj.convert_attribute_members(entry_attrs, *keys, **options)
 
         for callback in self.get_callbacks('post'):
@@ -2258,7 +2258,7 @@ class LDAPRemoveReverseMember(LDAPModReverseMember):
                 failed['member'][self.reverse_attr].append((attr, unicode(e)))
 
         # Update the member data.
-        entry_attrs = ldap.get_entry(dn, ['*'])
+        entry_attrs = ldap.get_entry(dn, attrs_list + ['objectclass'])
         self.obj.convert_attribute_members(entry_attrs, *keys, **options)
 
         for callback in self.get_callbacks('post'):
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to