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
>From d34c3726cd6b2f9dc7c206ce4edb9d1da405b8cf Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Thu, 11 Apr 2013 16:59:41 +0200
Subject: [PATCH] Add nfs:NONE to default PAC types only when needed

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
---
 install/updates/60-trusts.update         |  5 ---
 ipaserver/install/plugins/Makefile.am    |  1 +
 ipaserver/install/plugins/update_pacs.py | 57 ++++++++++++++++++++++++++++++++
 3 files changed, 58 insertions(+), 5 deletions(-)
 create mode 100644 ipaserver/install/plugins/update_pacs.py

diff --git a/install/updates/60-trusts.update b/install/updates/60-trusts.update
index f63651f7a6802e73c7f8a0deb75b5b964b090e98..1b25115402cb0371295525b4bcb7554d31478abc 100644
--- a/install/updates/60-trusts.update
+++ b/install/updates/60-trusts.update
@@ -73,11 +73,6 @@ replace:aci:'(targetattr = "userPassword || krbPrincipalKey || sambaLMPassword |
 dn: cn=ipaConfig,cn=etc,$SUFFIX
 addifnew: ipaKrbAuthzData: MS-PAC
 
-# Add authorization data type NONE for NFS because the hardcoded default was
-# removed.
-dn: cn=ipaConfig,cn=etc,$SUFFIX
-add: ipaKrbAuthzData: nfs:NONE
-
 # Fix typo in some installs in the spelling of ORDERING. They were added
 # with a typo which was silently dropped by 389-ds-base, so add in the
 # proper ordering syntax now.
diff --git a/ipaserver/install/plugins/Makefile.am b/ipaserver/install/plugins/Makefile.am
index a0c62ca702978138c714f53362c3adc1b404b33f..6a585327a4a57adfc9a2658974ca08393854ac3f 100644
--- a/ipaserver/install/plugins/Makefile.am
+++ b/ipaserver/install/plugins/Makefile.am
@@ -10,6 +10,7 @@ app_PYTHON = 			\
 	updateclient.py		\
 	update_services.py	\
 	update_anonymous_aci.py	\
+        update_pacs.py          \
 	$(NULL)
 
 EXTRA_DIST =			\
diff --git a/ipaserver/install/plugins/update_pacs.py b/ipaserver/install/plugins/update_pacs.py
new file mode 100644
index 0000000000000000000000000000000000000000..653456bb84d5464022024f5baaf4a7543f01f96f
--- /dev/null
+++ b/ipaserver/install/plugins/update_pacs.py
@@ -0,0 +1,57 @@
+# Authors:
+#   Tomas Babej <tba...@redhat.com>
+#
+# Copyright (C) 2013  Red Hat
+# see file 'COPYING' for use and warranty information
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from ipaserver.install.plugins import MIDDLE
+from ipaserver.install.plugins.baseupdate import PostUpdate
+from ipalib import api, errors
+from ipapython.dn import DN
+
+
+class update_pacs(PostUpdate):
+    """
+    Includes default nfs:None only if no nfs: PAC 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', [])
+        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']
+            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

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

Reply via email to