[Freeipa-devel] [PATCH] 80 Avoid ldapmodify error messages during ipa-adtrust-install

[Sumit Bose]

On Thu, Oct 04, 2012 at 12:39:07PM +0300, Alexander Bokovoy wrote:
> On Thu, 04 Oct 2012, Sumit Bose wrote:
> >On Thu, Oct 04, 2012 at 12:13:57PM +0300, Alexander Bokovoy wrote:
> >>On Thu, 04 Oct 2012, Sumit Bose wrote:
> >>>Hi,
> >>>
> >>>this patch tries to avoid the ldapmodiy error messages during
> >>>ipa-adtrust-install by checking if the related object already exists.
> >>>Fixes https://fedorahosted.org/freeipa/ticket/3012 .
> >>In general -- ACK for the approach. However, I wonder if you could create
> >>a helper method that would accept:
> >>  - ldif file name,
> >>  - cn component
> >>  - name of the plugin for the "already configured" message
> >>
> >>Then every __add_* method would call simply the helper with appropriate
> >>arguments.
> >
> >yes, I was thinking about the same, but I preferred this solution for
> >now instead of refactoring the current code. Additionally it would be
> >nice if the helper method would read the DN from the ldif file to make
> >the code more robust. Would you mind to open a trac ticket to refactor
> >this part of the code?
> I would disagree here. Reading DN from ldif file adds complexity for no
> real gain.
> 
> Refactoring this code is really simple -- your patch did already
> introduced bigger change than that refactoring would make.
> 
> If you aren't comfort with this suggestion, I can make the change
> myself. Having these repeated patterns is hurting my eyes :)

new version attached.

bye,
Sumit
> 
> -- 
> / Alexander Bokovoy
-------------- next part --------------
From f304c9ac9a1e1c46661531780f771409c7a2647c Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Thu, 4 Oct 2012 10:15:40 +0200
Subject: [PATCH] Avoid ldapmodify error messages during ipa-adtrust-install

Fixes https://fedorahosted.org/freeipa/ticket/3012
---
 ipaserver/install/adtrustinstance.py | 47 ++++++++++++++++++++++++++----------
 1 Datei ge?ndert, 34 Zeilen hinzugef?gt(+), 13 Zeilen entfernt(-)

diff --git a/ipaserver/install/adtrustinstance.py b/ipaserver/install/adtrustinstance.py
index 8822c2855eff3b9855914679ac7cedd016ea9185..7bfb6076d7ef5d4f6c16df85c64252d0aad062df 100644
--- a/ipaserver/install/adtrustinstance.py
+++ b/ipaserver/install/adtrustinstance.py
@@ -323,18 +323,37 @@ class ADTRUSTInstance(service.Service):
         conf_fd.write('config backend = registry\n')
         conf_fd.close()
 
+    def __add_plugin_conf(self, name, plugin_cn, ldif_file):
+        """
+        Add directory server plugin configuration if it not already
+        exists.
+        """
+        try:
+            plugin_dn = DN(('cn', plugin_cn), ('cn', 'plugins'),
+                           ('cn', 'config'))
+            self.admin_conn.getEntry(plugin_dn, ldap.SCOPE_BASE)
+            self.print_msg('%s plugin already configured, nothing to do' % name)
+        except errors.NotFound:
+            try:
+                self._ldap_mod(ldif_file, self.sub_dict)
+            except Exception:
+                pass
+
     def __add_cldap_module(self):
-        try:
-            self._ldap_mod("ipa-cldap-conf.ldif", self.sub_dict)
-        except:
-            pass
+        """
+        Add cldap directory server plugin configuration if it not already
+        exists.
+        """
+        self.__add_plugin_conf('CLDAP', 'ipa_cldap', 'ipa-cldap-conf.ldif')
 
     def __add_sidgen_module(self):
-        try:
-            self._ldap_mod("ipa-sidgen-conf.ldif", self.sub_dict)
-            self._ldap_mod("ipa-sidgen-task-conf.ldif", self.sub_dict)
-        except Exception:
-            pass
+        """
+        Add sidgen directory server plugin configuration and the related task
+        if they not already exist.
+        """
+        self.__add_plugin_conf('Sidgen', 'IPA SIDGEN', 'ipa-sidgen-conf.ldif')
+        self.__add_plugin_conf('Sidgen task', 'ipa-sidgen-task',
+                               'ipa-sidgen-task-conf.ldif')
 
     def __add_sids(self):
         """
@@ -347,10 +366,12 @@ class ADTRUSTInstance(service.Service):
             pass
 
     def __add_extdom_module(self):
-        try:
-            self._ldap_mod("ipa-extdom-extop-conf.ldif", self.sub_dict)
-        except:
-            pass
+        """
+        Add directory server configuration for the extdom extended operation
+        if it not already exists.
+        """
+        self.__add_plugin_conf('Extdom', 'ipa_extdom_extop',
+                               'ipa-extdom-extop-conf.ldif')
 
     def __write_smb_registry(self):
         template = os.path.join(ipautil.SHARE_DIR, "smb.conf.template")
-- 
1.7.11.4