On 02/06/15 10:24, Fraser Tweedale wrote:
On Mon, Jun 01, 2015 at 04:47:46PM +0200, Martin Basti wrote:
On 01/06/15 16:14, Rob Crittenden wrote:
Martin Basti wrote:
Fixes an issue caused by the latest installer patches pushed to master.

Patch attached.



The use of globals makes my skin crawl a bit, but since you're making
changes in here you should take a look at this ticket:
https://fedorahosted.org/freeipa/ticket/5042

rob
Hi Rob,

this is fix for that ticket, I missed the ticket somehow.

Thanks.
Martin^2

--
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
Fixes the problem for me, but I agree with Rob re globals - a
context manager would be much nicer.  Something like (pseudocode):

     @contextlib.context_manager
     def private_ccache():
         ... stuff currently in init_private_ccache()
         yield
         ... stuff currently in destroy_private_ccache()

Then in ipa-server-install main():

     with private_ccache:
        if not options.uninstall:
            server.install_check(options)
            server.install(options)
         else:
            server.uninstall_check(options)
            server.uninstall(options)

Cheers,
Fraser
Hello,

comments below:

1)
+    Str(
+        'memberprincipal',
+        label=_('Failed principals'),
+    ),
+    Str(
+        'ipaallowedtarget',
+        label=_('Failed targets'),
+    ),
+    Str(
+        'servicedelegationrule',
+        label=_('principal member'),
+    ),
Are these names correct?
# ipa servicedelegationrule-find
----------------------------------
1 service delegation rule matched
----------------------------------
  Delegation name: ipa-http-delegation
  Allowed Target: ipa-ldap-delegation-targets, ipa-cifs-delegation-targets
  Failed principals: HTTP/vm-093.example....@example.com


2)
+ pattern='^[a-zA-Z0-9_.][a-zA-Z0-9_.-]{0,252}[a-zA-Z0-9_.$-]?$',
+            pattern_errmsg='may only include letters, numbers, _, -, ., '
+                           'and a space inside',

This regex does not allow space inside
In [6]: print re.match(pattern, 'lalalala lalala')
None


3)
+            yield Str('%s*' % name, cli_name='%ss' % name, doc=doc,
+                      label=_('member %s') % name,
+                      csv=True, alwaysask=True)

IMHO CSV values should not be supported.
Honza told me, the option doesn't work anyway.

Patch with minor fixes attached.

I removed unused code and PEP8 complains


Martin^2
From 3d051d3ec13fe19e2e5246b28e848460538d81bf Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Tue, 2 Jun 2015 15:49:21 +0200
Subject: [PATCH] review service delegation

---
 ipalib/plugins/servicedelegation.py | 40 ++++++++++++++-----------------------
 1 file changed, 15 insertions(+), 25 deletions(-)

diff --git a/ipalib/plugins/servicedelegation.py b/ipalib/plugins/servicedelegation.py
index 434c446de590299b8a1c26344cbecd17f6a8ef3c..50b07dc8ad3112c0ee636b0d0ea7e0cd59365d9c 100644
--- a/ipalib/plugins/servicedelegation.py
+++ b/ipalib/plugins/servicedelegation.py
@@ -178,8 +178,7 @@ class servicedelegation_add_member(LDAPAddMember):
             name = self.member_names[attr]
             doc = self.member_param_doc % name
             yield Str('%s*' % name, cli_name='%ss' % name, doc=doc,
-                      label=_('member %s') % name,
-                      csv=True, alwaysask=True)
+                      label=_('member %s') % name, alwaysask=True)
 
     def get_member_dns(self, **options):
         """
@@ -187,7 +186,7 @@ class servicedelegation_add_member(LDAPAddMember):
         special handling since it is just a principal, not a
         full dn.
         """
-        return (dict(), dict())
+        return dict(), dict()
 
     def post_callback(self, ldap, completed, failed, dn, entry_attrs,
                       *keys, **options):
@@ -210,8 +209,8 @@ class servicedelegation_add_member(LDAPAddMember):
                 name = normalize_principal(name)
                 obj_dn = ldap_obj.get_dn(name)
                 try:
-                    s_attrs = ldap.get_entry(obj_dn, ['krbprincipalname'])
-                except errors.NotFound, e:
+                    ldap.get_entry(obj_dn, ['krbprincipalname'])
+                except errors.NotFound as e:
                     failed[attr][ldap_obj_name].append((name, unicode(e)))
                     continue
                 try:
@@ -219,22 +218,21 @@ class servicedelegation_add_member(LDAPAddMember):
                         members.append(name)
                     else:
                         raise errors.AlreadyGroupMember()
-                except errors.PublicError, e:
+                except errors.PublicError as e:
                     failed[attr][ldap_obj_name].append((name, unicode(e)))
                 else:
                     completed += 1
 
         if members:
-            if attr in entry_attrs:
-                entry_attrs[attr] += members
-            else:
-                entry_attrs[attr] = members
+            value = entry_attrs.setdefault(attr, [])
+            value.extend(members)
+
             try:
                 ldap.update_entry(entry_attrs)
             except errors.EmptyModlist:
                 pass
 
-        return (completed, dn)
+        return completed, dn
 
 
 class servicedelegation_remove_member(LDAPRemoveMember):
@@ -253,8 +251,7 @@ class servicedelegation_remove_member(LDAPRemoveMember):
             name = self.member_names[attr]
             doc = self.member_param_doc % name
             yield Str('%s*' % name, cli_name='%ss' % name, doc=doc,
-                      label=_('member %s') % name,
-                      csv=True, alwaysask=True)
+                      label=_('member %s') % name, alwaysask=True)
 
     def get_member_dns(self, **options):
         """
@@ -282,9 +279,9 @@ class servicedelegation_remove_member(LDAPRemoveMember):
                     ldap_obj = self.api.Object[ldap_obj_name]
                     try:
                         dns[attr][ldap_obj_name].append(ldap_obj.get_dn(name))
-                    except errors.PublicError, e:
+                    except errors.PublicError as e:
                         failed[attr][ldap_obj_name].append((name, unicode(e)))
-        return (dns, failed)
+        return dns, failed
 
     def post_callback(self, ldap, completed, failed, dn, entry_attrs,
                       *keys, **options):
@@ -295,7 +292,6 @@ class servicedelegation_remove_member(LDAPRemoveMember):
         ldap = self.obj.backend
         ldap_obj_name = self.obj.name
         attr = 'memberprincipal'
-        members = []
         failed[attr][ldap_obj_name] = []
         names = options.get(self.member_names[attr], [])
         if names:
@@ -308,7 +304,7 @@ class servicedelegation_remove_member(LDAPRemoveMember):
                         entry_attrs[attr].remove(name)
                     else:
                         raise errors.NotGroupMember()
-                except errors.PublicError, e:
+                except errors.PublicError as e:
                     failed[attr][ldap_obj_name].append((name, unicode(e)))
                 else:
                     completed += 1
@@ -318,7 +314,7 @@ class servicedelegation_remove_member(LDAPRemoveMember):
         except errors.EmptyModlist:
             pass
 
-        return (completed, dn)
+        return completed, dn
 
 
 @register()
@@ -358,9 +354,6 @@ class servicedelegationrule_del(LDAPDelete):
 
     def pre_callback(self, ldap, dn, *keys, **options):
         assert isinstance(dn, DN)
-        delegation_attrs = self.obj.methods.show(
-            self.obj.get_primary_key_from_dn(dn), all=True
-        )['result']
         if keys[0] in PROTECTED_CONSTRAINT_RULES:
             raise errors.ProtectedEntryError(
                 label=_(u'service delegation rule'),
@@ -456,9 +449,6 @@ class servicedelegationtarget_del(LDAPDelete):
 
     def pre_callback(self, ldap, dn, *keys, **options):
         assert isinstance(dn, DN)
-        delegation_attrs = self.obj.methods.show(
-            self.obj.get_primary_key_from_dn(dn), all=True
-        )['result']
         if keys[0] in PROTECTED_CONSTRAINT_TARGETS:
             raise errors.ProtectedEntryError(
                 label=_(u'service delegation target'),
@@ -504,7 +494,7 @@ class servicedelegationtarget_find(LDAPSearch):
         sfilter = ldap.combine_filters(
             (term_filter, attr_filter), rules=ldap.MATCH_ALL
         )
-        return (sfilter, base_dn, ldap.SCOPE_ONELEVEL)
+        return sfilter, base_dn, ldap.SCOPE_ONELEVEL
 
 
 @register()
-- 
2.1.0

-- 
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