On 04/15/2013 12:32 PM, Tomas Babej wrote:
On 04/12/2013 04:52 PM, Petr Viktorin wrote:
On 04/12/2013 04:10 PM, Tomas Babej wrote:
Hi,

We need to add nfs:NONE as a default PAC type only if there's no
other default PAC type for nfs. Adds a update plugin which
determines whether default PAC type for nfs is set and adds
nfs:NONE PAC type accordingly.

https://fedorahosted.org/freeipa/ticket/3555

Tomas

Looks good, works well, see a few nitpicks below.

[...]
diff --git a/ipaserver/install/plugins/update_pacs.py
b/ipaserver/install/plugins/update_pacs.py
[...]
+from ipapython.ipa_log_manager import *

Please don't use star imports in new code. Only import what you need
(that is, remove this import entirely).

+class update_pacs(PostUpdate):
+    """
+    Includes default nfs:None only if no nfs: entry present in
ipakrbauthzdata.
+    """
+
+    order = MIDDLE
+
+    def execute(self, **options):
+        ldap = self.obj.backend
+
+        try:
+            dn = DN('cn=ipaConfig', 'cn=etc', api.env.basedn)
+            entry = ldap.get_entry(dn, ['ipakrbauthzdata'])
+            pacs = entry.get('ipakrbauthzdata')
+
+            if pacs is None:

This shouldn't happen (ipaKrbAuthzData gets a default in previous
updates). It's not necessary to complicate the code, `pacs =
entry.get('ipakrbauthzdata', [])` would do.
Yes, this was probably overcautious.

+                self.log.warning('No ipakrbauthzdata attribute found.')
+                return (False, False, [])

+        except errors.NotFound:
+            self.log.warning('Error retrieving: %s' % str(dn))
+            return (False, False, [])
+
+        nfs_pac_set = any(pac.startswith('nfs:') for pac in pacs)
+
+        if not nfs_pac_set:
+            self.log.debug('Adding nfs:None to default PAC types')
+            updated_pacs = pacs + [u'nfs:NONE']
+            update = dict(ipakrbauthzdata=updated_pacs)
+            ldap.update_entry(dn, update)

This will work, but the preferred form for updates is now:
    entry['ipakrbauthzdata'] = updated_pacs
    ldap.update_entry(entry)

+        else:
+            self.log.debug('PAC for nfs is already set, not adding
nfs:NONE.')
+
+        return (False, False, [])
+
+api.register(update_pacs)
-- 1.8.1.4

Please also add the new update file to Makefile.am.


Thank you for the review. Updated patch attached.

Tomas

ACK

--
PetrĀ³

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

Reply via email to