Martin Kosek wrote:
On Tue, 2011-08-30 at 23:50 -0400, Rob Crittenden wrote:
This was spawned by another recent patch I pushed that showed netgroups
that a hostgroup is a member of. We want to suppress the automatic
netgroup that is created with hostgroups.

rob

NACK. I see several issues with the patch:

1) Lint problem:
ipalib/plugins/hostgroup.py:107: [E0602, hostgroup.suppress_netgroup_memberof] 
Undefined variable 'NotFound'

2) When there is not hostgroup, `ipa hostgroup-find` throws an
exception:
[Wed Aug 31 04:36:11 2011] [error] ipa: ERROR: non-public: UnboundLocalError: 
local variable 'dn' referenced before assignment
[Wed Aug 31 04:36:11 2011] [error] Traceback (most recent call last):
[Wed Aug 31 04:36:11 2011] [error]   File 
"/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 223, in 
wsgi_execute
[Wed Aug 31 04:36:11 2011] [error]     result = self.Command[name](*args, 
**options)
[Wed Aug 31 04:36:11 2011] [error]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 432, in __call__
[Wed Aug 31 04:36:11 2011] [error]     ret = self.run(*args, **options)
[Wed Aug 31 04:36:11 2011] [error]   File 
"/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 738, in run
[Wed Aug 31 04:36:11 2011] [error]     return self.execute(*args, **options)
[Wed Aug 31 04:36:11 2011] [error]   File 
"/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line 1596, in 
execute
[Wed Aug 31 04:36:11 2011] [error]     callback(ldap, entries, truncated, 
*args, **options)
[Wed Aug 31 04:36:11 2011] [error]   File 
"/usr/lib/python2.7/site-packages/ipalib/plugins/hostgroup.py", line 163, in 
post_callback
[Wed Aug 31 04:36:11 2011] [error]     return dn
[Wed Aug 31 04:36:11 2011] [error] UnboundLocalError: local variable 'dn' 
referenced before assignment

3) JR's automember tests that were pushed today will need to have
memberof_netgroup removed too.

Martin


All issues addressed.

rob
>From 79ce49eb3ce5d35b22efe9c8019e66ff56686ee5 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcrit...@redhat.com>
Date: Tue, 30 Aug 2011 18:38:22 -0400
Subject: [PATCH] Suppress managed netgroups from showing as memberof hostgroups.

By design these managed netgroups are not supposed to show unless you
specifically want to see them.

https://fedorahosted.org/freeipa/ticket/1738
---
 ipalib/plugins/hostgroup.py                 |   50 ++++++++++++++++++++++++--
 tests/test_xmlrpc/test_automember_plugin.py |   12 +------
 tests/test_xmlrpc/test_hostgroup_plugin.py  |    7 ----
 tests/test_xmlrpc/test_nesting.py           |    5 ---
 tests/test_xmlrpc/test_netgroup_plugin.py   |    1 -
 5 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/ipalib/plugins/hostgroup.py b/ipalib/plugins/hostgroup.py
index d75f381..0d69d09 100644
--- a/ipalib/plugins/hostgroup.py
+++ b/ipalib/plugins/hostgroup.py
@@ -19,7 +19,8 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from ipalib.plugins.baseldap import *
-from ipalib import api, Int, _, ngettext
+from ipalib import api, Int, _, ngettext, errors
+from ipalib.dn import DN
 
 __doc__ = _("""
 Groups of hosts.
@@ -88,6 +89,24 @@ class hostgroup(LDAPObject):
         ),
     )
 
+    def suppress_netgroup_memberof(self, dn, entry_attrs):
+        """
+        We don't want to show managed netgroups so remove them from the
+        memberOf list.
+        """
+        if 'memberof' in entry_attrs:
+            hgdn = DN(dn)
+            for member in entry_attrs['memberof']:
+                ngdn = DN(member)
+                if ngdn['cn'] == hgdn['cn']:
+                    try:
+                        netgroup = api.Command['netgroup_show'](ngdn['cn'], all=True)['result']
+                        if self.has_objectclass(netgroup['objectclass'], 'mepmanagedentry'):
+                            entry_attrs['memberof'].remove(member)
+                            return
+                    except errors.NotFound:
+                        pass
+
 api.register(hostgroup)
 
 
@@ -97,9 +116,11 @@ class hostgroup_add(LDAPCreate):
     msg_summary = _('Added hostgroup "%(value)s"')
 
     def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
-        if self.api.env.wait_for_attr:
-            newentry = wait_for_value(ldap, dn, 'objectclass', 'mepOriginEntry')
-            entry_from_entry(entry_attrs, newentry)
+        # Always wait for the associated netgroup to be created so we can
+        # be sure to ignore it in memberOf
+        newentry = wait_for_value(ldap, dn, 'objectclass', 'mepOriginEntry')
+        entry_from_entry(entry_attrs, newentry)
+        self.obj.suppress_netgroup_memberof(dn, entry_attrs)
 
         return dn
 
@@ -120,6 +141,10 @@ class hostgroup_mod(LDAPUpdate):
 
     msg_summary = _('Modified hostgroup "%(value)s"')
 
+    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        self.obj.suppress_netgroup_memberof(dn, entry_attrs)
+        return dn
+
 api.register(hostgroup_mod)
 
 
@@ -131,22 +156,39 @@ class hostgroup_find(LDAPSearch):
         '%(count)d hostgroup matched', '%(count)d hostgroups matched', 0
     )
 
+    def post_callback(self, ldap, entries, truncated, *args, **options):
+        for entry in entries:
+            (dn, entry_attrs) = entry
+            self.obj.suppress_netgroup_memberof(dn, entry_attrs)
+
 api.register(hostgroup_find)
 
 
 class hostgroup_show(LDAPRetrieve):
     __doc__ = _('Display information about a hostgroup.')
 
+    def post_callback(self, ldap, dn, entry_attrs, *keys, **options):
+        self.obj.suppress_netgroup_memberof( dn, entry_attrs)
+        return dn
+
 api.register(hostgroup_show)
 
 
 class hostgroup_add_member(LDAPAddMember):
     __doc__ = _('Add members to a hostgroup.')
 
+    def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options):
+        self.obj.suppress_netgroup_memberof(dn, entry_attrs)
+        return (completed, dn)
+
 api.register(hostgroup_add_member)
 
 
 class hostgroup_remove_member(LDAPRemoveMember):
     __doc__ = _('Remove members from a hostgroup.')
 
+    def post_callback(self, ldap, completed, failed, dn, entry_attrs, *keys, **options):
+        self.obj.suppress_netgroup_memberof(dn, entry_attrs)
+        return (completed, dn)
+
 api.register(hostgroup_remove_member)
diff --git a/tests/test_xmlrpc/test_automember_plugin.py b/tests/test_xmlrpc/test_automember_plugin.py
index 342bc21..1241bd6 100644
--- a/tests/test_xmlrpc/test_automember_plugin.py
+++ b/tests/test_xmlrpc/test_automember_plugin.py
@@ -136,7 +136,6 @@ class test_automember(Declarative):
                     description=[u'Test desc'],
                     objectclass=objectclasses.hostgroup,
                     ipauniqueid=[fuzzy_uuid],
-                    memberof_netgroup=[u'hostgroup1'],
                     mepmanagedentry=['cn=%s,cn=ng,cn=alt,%s' % (hostgroup1, api.env.basedn)],
                     dn=u'cn=%s,cn=hostgroups,cn=accounts,%s' % (hostgroup1, api.env.basedn),
                 ),
@@ -157,7 +156,6 @@ class test_automember(Declarative):
                     description=[u'Test desc'],
                     objectclass=objectclasses.hostgroup,
                     ipauniqueid=[fuzzy_uuid],
-                    memberof_netgroup=[u'hostgroup2'],
                     mepmanagedentry=['cn=%s,cn=ng,cn=alt,%s' % (hostgroup2, api.env.basedn)],
                     dn=u'cn=%s,cn=hostgroups,cn=accounts,%s' % (hostgroup2, api.env.basedn),
                 ),
@@ -178,7 +176,6 @@ class test_automember(Declarative):
                     description=[u'Test desc'],
                     objectclass=objectclasses.hostgroup,
                     ipauniqueid=[fuzzy_uuid],
-                    memberof_netgroup=[u'hostgroup3'],
                     mepmanagedentry=['cn=%s,cn=ng,cn=alt,%s' % (hostgroup3, api.env.basedn)],
                     dn=u'cn=%s,cn=hostgroups,cn=accounts,%s' % (hostgroup3, api.env.basedn),
                 ),
@@ -199,7 +196,6 @@ class test_automember(Declarative):
                     description=[u'Test desc'],
                     objectclass=objectclasses.hostgroup,
                     ipauniqueid=[fuzzy_uuid],
-                    memberof_netgroup=[u'hostgroup4'],
                     mepmanagedentry=['cn=%s,cn=ng,cn=alt,%s' % (hostgroup4, api.env.basedn)],
                     dn=u'cn=%s,cn=hostgroups,cn=accounts,%s' % (hostgroup4, api.env.basedn),
                 ),
@@ -240,7 +236,6 @@ class test_automember(Declarative):
                     description=[u'Default test desc'],
                     objectclass=objectclasses.hostgroup,
                     ipauniqueid=[fuzzy_uuid],
-                    memberof_netgroup=[u'defaulthostgroup1'],
                     mepmanagedentry=['cn=%s,cn=ng,cn=alt,%s' % (defaulthostgroup1, api.env.basedn)],
                     dn=u'cn=%s,cn=hostgroups,cn=accounts,%s' % (defaulthostgroup1, api.env.basedn),
                 ),
@@ -998,7 +993,6 @@ class test_automember(Declarative):
                     'member_host': [u'%s' % fqdn1],
                     'cn': [hostgroup1],
                     'description': [u'Test desc'],
-                    'memberof_netgroup': [u'hostgroup1'],
                 },
             ),
         ),
@@ -1015,7 +1009,6 @@ class test_automember(Declarative):
                     'member_host': [u'%s' % fqdn2],
                     'cn': [defaulthostgroup1],
                     'description': [u'Default test desc'],
-                    'memberof_netgroup': [u'defaulthostgroup1'],
                 },
             ),
         ),
@@ -1032,7 +1025,6 @@ class test_automember(Declarative):
                     'member_host': [u'%s' % fqdn3],
                     'cn': [hostgroup2],
                     'description': [u'Test desc'],
-                    'memberof_netgroup': [u'hostgroup2'],
                 },
             ),
         ),
@@ -1049,7 +1041,6 @@ class test_automember(Declarative):
                     'member_host': [u'%s' % fqdn4],
                     'cn': [hostgroup3],
                     'description': [u'Test desc'],
-                    'memberof_netgroup': [u'hostgroup3'],
                 },
             ),
         ),
@@ -1066,9 +1057,8 @@ class test_automember(Declarative):
                     'member_host': [u'%s' % fqdn5],
                     'cn': [hostgroup4],
                     'description': [u'Test desc'],
-                    'memberof_netgroup': [u'hostgroup4'],
                 },
             ),
         ),
 
-    ]
\ No newline at end of file
+    ]
diff --git a/tests/test_xmlrpc/test_hostgroup_plugin.py b/tests/test_xmlrpc/test_hostgroup_plugin.py
index 6c3b0a4..e0d1158 100644
--- a/tests/test_xmlrpc/test_hostgroup_plugin.py
+++ b/tests/test_xmlrpc/test_hostgroup_plugin.py
@@ -83,7 +83,6 @@ class test_hostgroup(Declarative):
                     objectclass=objectclasses.hostgroup,
                     description=[u'Test hostgroup 1'],
                     ipauniqueid=[fuzzy_uuid],
-                    memberof_netgroup=[hostgroup1],
                     mepmanagedentry=lambda x: [DN(i) for i in x] == \
                         [DN(('cn',hostgroup1),('cn','ng'),('cn','alt'),
                             api.env.basedn)],
@@ -147,7 +146,6 @@ class test_hostgroup(Declarative):
                     'cn': [hostgroup1],
                     'description': [u'Test hostgroup 1'],
                     'member_host': [fqdn1],
-                    'memberof_netgroup': [hostgroup1],
                 },
             ),
         ),
@@ -164,7 +162,6 @@ class test_hostgroup(Declarative):
                     'member_host': [u'testhost1.%s' % api.env.domain],
                     'cn': [hostgroup1],
                     'description': [u'Test hostgroup 1'],
-                    'memberof_netgroup': [hostgroup1],
                 },
             ),
         ),
@@ -183,7 +180,6 @@ class test_hostgroup(Declarative):
                         'member_host': [u'testhost1.%s' % api.env.domain],
                         'cn': [hostgroup1],
                         'description': [u'Test hostgroup 1'],
-                        'memberof_netgroup': [hostgroup1],
                     },
                 ],
             ),
@@ -202,7 +198,6 @@ class test_hostgroup(Declarative):
                     cn=[hostgroup1],
                     description=[u'Updated hostgroup 1'],
                     member_host=[u'testhost1.%s' % api.env.domain],
-                    memberof_netgroup=[hostgroup1],
                 ),
             ),
         ),
@@ -219,7 +214,6 @@ class test_hostgroup(Declarative):
                     'member_host': [u'testhost1.%s' % api.env.domain],
                     'cn': [hostgroup1],
                     'description': [u'Updated hostgroup 1'],
-                    'memberof_netgroup': [hostgroup1],
                 },
             ),
         ),
@@ -242,7 +236,6 @@ class test_hostgroup(Declarative):
                     'dn': lambda x: DN(x) == dn1,
                     'cn': [hostgroup1],
                     'description': [u'Updated hostgroup 1'],
-                    'memberof_netgroup': [hostgroup1],
                 },
             ),
         ),
diff --git a/tests/test_xmlrpc/test_nesting.py b/tests/test_xmlrpc/test_nesting.py
index 31525cd..cb2d1d0 100644
--- a/tests/test_xmlrpc/test_nesting.py
+++ b/tests/test_xmlrpc/test_nesting.py
@@ -705,7 +705,6 @@ class test_nesting(Declarative):
                     objectclass=objectclasses.hostgroup,
                     description=[u'Test hostgroup 1'],
                     ipauniqueid=[fuzzy_uuid],
-                    memberof_netgroup=[hostgroup1],
                     mepmanagedentry=lambda x: [DN(i) for i in x] == \
                         [DN(('cn',hostgroup1),('cn','ng'),('cn','alt'),
                             api.env.basedn)],
@@ -728,7 +727,6 @@ class test_nesting(Declarative):
                     objectclass=objectclasses.hostgroup,
                     description=[u'Test hostgroup 2'],
                     ipauniqueid=[fuzzy_uuid],
-                    memberof_netgroup=[hostgroup2],
                     mepmanagedentry=lambda x: [DN(i) for i in x] == \
                         [DN(('cn',hostgroup2),('cn','ng'),('cn','alt'),
                             api.env.basedn)],
@@ -755,7 +753,6 @@ class test_nesting(Declarative):
                     'cn': [hostgroup2],
                     'description': [u'Test hostgroup 2'],
                     'member_host': [fqdn1],
-                    'memberof_netgroup': [hostgroup2],
                 },
             ),
         ),
@@ -779,7 +776,6 @@ class test_nesting(Declarative):
                     'cn': [hostgroup1],
                     'description': [u'Test hostgroup 1'],
                     'member_hostgroup': [hostgroup2],
-                    'memberof_netgroup': [hostgroup1],
                 },
             ),
         ),
@@ -795,7 +791,6 @@ class test_nesting(Declarative):
                     'dn': lambda x: DN(x) == hgdn1,
                     'memberindirect_host': [u'testhost1.%s' % api.env.domain],
                     'member_hostgroup': [hostgroup2],
-                    'memberof_netgroup': [hostgroup1],
                     'cn': [hostgroup1],
                     'description': [u'Test hostgroup 1'],
                 },
diff --git a/tests/test_xmlrpc/test_netgroup_plugin.py b/tests/test_xmlrpc/test_netgroup_plugin.py
index f28c667..9194b54 100644
--- a/tests/test_xmlrpc/test_netgroup_plugin.py
+++ b/tests/test_xmlrpc/test_netgroup_plugin.py
@@ -188,7 +188,6 @@ class test_netgroup(Declarative):
                     cn=[hostgroup1],
                     objectclass=objectclasses.hostgroup,
                     description=[u'Test hostgroup 1'],
-                    memberof_netgroup=[hostgroup1],
                     mepmanagedentry=lambda x: [DN(i) for i in x] == \
                         [DN(('cn',hostgroup1),('cn','ng'),('cn','alt'),
                             api.env.basedn)],
-- 
1.7.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to